Re: [Boost-users] [signals] thread_safe_signals feature-complete

From: Timmo Stange (ts_at_[hidden])
Date: 2007-02-17 13:56:28

Frank Mori Hess wrote:

> Hi all,
> 
> I've finished thread_safe_signals.  To get thread safety you have to 
> include an additional header file thread_safe_signals/multi_threaded.hpp 
> and set the last template parameter to boost::signals::multi_threaded.

Nice. I only had a few minutes yet to look at the newest version, so
I might not have a good grasp on the bigger picture, but there are
a few things I have difficulties to understand.

This is from your signal operator()(...):

   shared_ptr<connection_list_type> localConnectionBodies;
   shared_ptr<const combiner_type> local_combiner;
   typename connection_list_type::iterator it;
   {
     typename mutex_type::scoped_lock listLock(_mutex);
     // only clean up if it is safe to do so
     if(_connectionBodies.use_count() == 1)
       nolock_cleanup_connections(false);
     localConnectionBodies = _connectionBodies;
     /* make a local copy of _combiner while holding mutex, so we
     are thread safe against the combiner getting modified by
     set_combiner()*/
     local_combiner = _combiner;
   }
   [...followed by the invocation...]

The localConnectionBodies is a shared pointer to the actual
slot list and not a full copy, so I don't understand how this
is going to be thread-safe against concurrent invocations. The
mutex is released after this compound statement and the
invocation traverses a list that might be modified by the
"nolock_cleanup_connections(false);" in a concurrent call to
the same function.

Similar things appear elsewhere in the signal template.

In the slot_call_iterator template I don't understand why
you call lockNextCallable in dereference() and call it
twice (instead of just once after incrementing the
iterator) in increment(). That function may advance the
iterator, which can do you no good in dereference() and
is unnecessary there, as far as I can tell.

It also doesn't seem like the slot_call_iterator skips
disconnected slots.

Regards

Timmo Stange