Opened 10 years ago

Last modified 10 years ago

#7015 new Bugs

boost::signals::connection with shared_ptr to connected signal

Reported by: Nick Mayer <nick.mayer@…> Owned by: Douglas Gregor
Milestone: To Be Determined Component: signals
Version: Boost 1.49.0 Severity: Problem
Keywords: signals connection shared_ptr Cc:

Description

If you create a connection with a shared_ptr that controls the lifespan of the signal an access violation occurs on disconnection if that is the last remaining reference to the object.

class Child : public boost::signals::trackable
{
public:
    Child(){}

    virtual ~Child(){}

    void callback( int a, int b )
    {
        printf( "Child::callback( %d, %d )\n", a, b );
    }

    boost::signal<void(int)> mSignal;
};

int main(int argc, char* argv[])
{
    boost::shared_ptr<Child> c( new Child );
    boost::signals::connection con;
    con = c->mSignal.connect( boost::bind( &Child::callback, c, 1, _1 ) );
    c->mSignal( 5 );
    c.reset();

    // The connection is the only thing keeping it alive. When we disconnect
    // it will also destroy the signal. This causes a memory access violation
    // when the signal containing the list of connections gets deleted while
    // removing an item from that list.
    con.disconnect();
}

The fix is to prevent the slot from going out of scope during the act of disconnecting from the signal.

diff --git a/libs/signals/src/signal_base.cpp b/libs/signals/src/signal_base.cpp
index 759672d..918e811 100644
--- a/libs/signals/src/signal_base.cpp
+++ b/libs/signals/src/signal_base.cpp
@@ -143,8 +143,14 @@ namespace boost {
             self->flags.delayed_disconnect = true;
           }
           else {
             // Just remove the slot now, it's safe
+            // There is a chance that the binding has the last reference to
+            // the signal, and destroying it will destroy the signal. To prevent
+            // this from causing a problem keep a local copy of the binding
+            // during it's removal from the slot list.
+            boost::any keep_alive = (*slot)->second;
             self->slots_.erase(*slot);
+            return;
           }
         }
       }

Attachments (1)

signal_base.patch (1.1 KB ) - added by Nick Mayer <nick.mayer@…> 10 years ago.
Patch in the description of the issue

Download all attachments as: .zip

Change History (2)

by Nick Mayer <nick.mayer@…>, 10 years ago

Attachment: signal_base.patch added

Patch in the description of the issue

comment:1 by anonymous, 10 years ago

Any ideas on what would need to be done to get this patch accepted?

Note: See TracTickets for help on using tickets.