Memory leak using multiple boost::connect on single slot_type

Question

I'm using boost::signals and leaking memory when I try to connect multiple signals to a single slot_type. I've seen this same leak reported on various forums, but can't find any that mention the correct way to do this, or any workaround.

What I am trying to do:

I am trying to pass the result of boost::bind() into a function. In this function, I want to connect multiple signals to that result. The first connect works fine, but every connect after the first will leak a handle.

Here is some sample code:

typedef boost::signal0<void> LeakSignalType;

class CalledClass
{
   /* ... */
   void connectToSlots(LeakSignalType::slot_type &aSlot)
   {
        LeakSignalType *sig;
        std::list<LeakSignalType*> sigList;
        std::list<LeakSignalType*>::iterator sigIter;

        for(int i = 0; i < 50; i++)
        {
            /*Connect signals to the passed slot */
            sig = new LeakSignalType;
            sig->connect(aSlot);
            sigList.push_back(sig);
        }
        for(sigIter = sigList.begin(); sigIter != sigList.end(); sigIter++)
        {
            /* Undo everything we just did */
            delete *sigIter;
        }
        sigList.clear();
        /*Everything should be cleaned up now */
   }
   /* ... */
}

class CallingClass : public boost::signals::trackable
{
   CalledClass calledInstance;
   /* ... */
   void boundFunction(int i)
   {
   /*Do Something*/
   }

   void connectSignals()
   {
       calledInstance.connectToSlots(boost::bind( &CallingClass::boundFunction, this, 1));
   }
   /* ... */
};

Now call CallingClass::connectSignals().

I expect that the call to connectToSlots will connect 50 signals to a single slot, then disconnect and clean up all of those signals. What actually happens is that 1 signal completely cleans up, then the remaining 49 partially clean up, but leak some memory.

What is the correct way to pass a slot into a function to use multiple times? Any help would be appreciated.

Thanks, Chris

Answer 1

I'm pretty sure it's a bug. If you collapse it down to a tiny example, e.g.:

void boundFunction(int) { }
typedef boost::signal0<void> LeakSignalType;
LeakSignalType::slot_type aSlot = boost::bind(&::boundFunction, 1);
LeakSignalType sig1, sig2;
sig1.connect(aSlot);
sig2.connect(aSlot);

and trace the allocations, you'll find that one object (a boost::signals::detail::signal_base_impl::iterator) allocated at line 75 of boost/lib/signals/src/signal_base.cpp is not freed up.

// Allocate storage for an iterator that will hold the point of
// insertion of the slot into the list. This is used to later remove
// the slot when it is disconnected.
std::auto_ptr<iterator> saved_iter(new iterator);

On the first connect, this iterator is attached to a fresh connection object, where signal_data is NULL:

data->watch_bound_objects.get_connection()->signal_data =
  saved_iter.release();

On the second connect, however, the same connection object is reused, and the same line blindly overwrites the original pointer value. The second object is cleaned up, but the first is not.

As verification, a breakpoint in signal_base_impl::slot_disconnected, the only place where signal_data is cleaned up, is only triggered once.

I tracked this down in 1.39.0, but it looks like it's the same in 1.40.0.

You could modify boost::signals::detail::signal_base_impl::connect_slot to clean up any previous iterator value it finds in the signal_data field of an existing connection, if you're comfortable making such a change and running a custom build of Boost.

It might be better to just make sure you're only setting these up a fixed number of times and live with a few small memory leaks that you know won't grow over time.

Update:

I was going to submit this to the Boost bug tracker, but it's already there. This is a much smaller test case, however.

https://svn.boost.org/trac/boost/ticket/738

Opened 3 years ago, not assigned to any milestone :-[

Answer 2

For other people's reference, I am having some luck maintaining my own copy of signal_data, and deleting it before deleting the signal. Don't know of any side effects, YMMV.

Something like this:

typedef boost::signal0<void> LeakSignalType;
struct LeakSignalStruct
{
    LeakSignalType  signal;
    /* A pointer to keep track of the pointer Boost loses */
    boost::signals::detail::named_slot_map_iterator *signal_data;
};

class CalledClass
{
   /* ... */
   void connectToSlots(LeakSignalType::slot_type &aSlot)
   {
        LeakSignalStruct *sig;
        std::list<LeakSignalStruct*> sigList;
        std::list<LeakSignalStruct*>::iterator sigIter;

        for(int i = 0; i < 50; i++)
        {
            /*Connect signals to the passed slot */
            sig = new LeakSignalStruct;
            sig->connect(aSlot);
            /* Make a backup of the reference that Boost will lose */
            sig->signal_data = (boost::signals::detail::named_slot_map_iterator*)connection.get_connection()->signal_data;
            sigList.push_back(sig);
        }

        /* Boost remembers the last signal_data and will delete it itself,
           so we better lose our reference to avoid double-delete */
        sig->signal_data = NULL;

        for(sigIter = sigList.begin(); sigIter != sigList.end(); sigIter++)
        {
            /* Undo everything we just did */
            /* Boost lost this reference, so we delete it ourselves */
            delete (*sigIter)->signal_data;
            delete *sigIter;
        }
        sigList.clear();
        /*Everything should be cleaned up now */
   }
   /* ... */
};

class CallingClass : public boost::signals::trackable
{
   CalledClass calledInstance;
   /* ... */
   void boundFunction(int i)
   {
   /*Do Something*/
   }

   void connectSignals()
   {
       calledInstance.connectToSlots(boost::bind( &CallingClass::boundFunction, this, 1));
   }
   /* ... */
};