On Wed, 5 May 2004, Eric Niebler wrote:
Douglas Gregor wrote:
projects, but the lack of thread-safety is a concern for us. What issues are preventing Boost.Signals from being made threadsafe?
The biggest issue is granularity. Do we use signal-level locking or connection/slot-level locking? The former requires few locks, but constrains the coding style quite a bit, whereas the latter is going to incur a bit of overhead (a lock for every slot call, disconnect, etc.).
I was thinking there would be one lock object per signal object.
This is the most intuitive way, and probably the right way.
I'm not sure I understand the other option you mention. Could you explain a bit?
Sure. I'm a bit concerned about the trackable issue, because deletion of a trackable object can mean that lots of connections (from many different signals) can be disconnected at once. So we could have the scenario where: Signal A has connections to slots a and b, both of which reference trackable object o. Signal B has connections to slots c and d, both of which reference trackable object o. We could be executing in Signal A and in Signal B simultaneously; then, slot c or d deletes object o (causing slots a, b, c, d to be disconnected). It's okay in Signal A--Signals was design for this--but can we be sure that Signal B won't fail? I think we can't, but we might be able to get close if we had locks in each "connection" object, and locked all connections involved with a slot before calling it. This is massive overhead, though, so I think I should just drop the notion.
Regardless, locking behavior should be made a policy. That way, users who are not interested in paying for thread-safety are accomodated.
Agreed. I was actually just about to prototype a threading policy for Function, as a warm-up for Signals.
I've been giving this issue some thought, and I think I can see a way to make this thread-safe. Here's how:
1) signal::operator() grabs the lock 2) signal::operator() passes the slot iterators to combiner::operator() 3) slot_iterator::operator* does the following: i) copies the slot into a local
Might not actually need this step, because the slot itself cannot be deleted at this point.
ii) releases the lock iii) calls the slot through the local iv) reaquires the lock.
Ah, clever! Good for asynchronous slot invocation as well, if we ever care about that.
4) slot_iterator::operator++ skips any nulled-out slots (note that it always executes when the lock is held)
... and this part is already implemented.
5) combiner::operator() returns and signal::operator() releases the lock.
Now, you must be careful in signal::connect() and signal::disconnect() to do nothing that would invalidate the slot iterators. Suppose they were std::list::iterators, and that signal::disconnect() merely nulled out slots instead of removing them from the list. Then this would work. But it needs to be documented that slot_iterators are only valid for the duration of combiner::operator(). (That is, you can't copy them into a global and use them after the lock has been released.)
The good news is that all of this special work to avoid invalidating slot iterators has already been done to deal with the recursive deletion case. I think it is documented that slot_call_iterators are only valid within the combiner, but I could have omitted that.
What do you think?
Sounds very, very promising. Doug