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