boost::signal thread safety (implied question: is boost::function thread safe ?)
Hello, Well, the subject says it all ;) This question comes because boost::signal's documentation says it's not thread safe, but I have trouble finding exactly why it isn't. Let's look the following example: Each thread is connected to the same signal, then the signal is emmited, and each threads receives the message.... Why would this not be thread safe ? Of course in this code the issue would be that I don't mutex the "finished" variable but that's another story. #include <iostream> #include <boost/bind.hpp> #include <boost/thread.hpp> #include <boost/signals.hpp> struct thread1 { void operator()() { finished = false; while(!finished); } void slot() { finished = true; } bool finished; }; struct thread2 { void operator()() { finished = false; while(!finished); } void slot() { finished = true; } bool finished; }; int main() { boost::signal<void ()> sig; thread1 t1; thread2 t2; sig.connect(boost::bind(&thread1::slot, &t1)); sig.connect(boost::bind(&thread2::slot, &t2)); boost::thread trd1(boost::ref(t1)); boost::thread trd2(boost::ref(t2)); usleep(10); sig(); trd1.join(); trd2.join(); return 0; } Thank you for insight. Philippe
Philippe Vaucher wrote:
Hello,
Well, the subject says it all ;) This question comes because boost::signal's documentation says it's not thread safe, but I have trouble finding exactly why it isn't.
Let's look the following example:
Each thread is connected to the same signal, then the signal is emmited, and each threads receives the message.... Why would this not be thread safe ?
You use 'thread' in a very lose way here. There is a 'thread' as seen by the OS, i.e. an execution entity processing some code, and there is are two 'thread' types ('thread1' and 'thread2') you use. In your description above you seem to refer to those types as 'threads', not to the execution entities. The point is, the 'threads of execution' don't 'receive the message', as the 'slot()' methods are executed in the main thread. However, as you check the value of 'finished' in other 'threads of execution', you have to protect access to it by a mutex. For example: struct thread1 { thread1() : finished_(false) {} void operator()() { while (true) { scoped_lock lock(mutex_); if (finished_) return; } } void slot() { scoped_lock lock(mutex_); finished_ = true; } bool fininshed_; mutex mutex_; }; HTH, Stefan -- ...ich hab' noch einen Koffer in Berlin...
The point is, the 'threads of execution' don't 'receive the message', as the 'slot()' methods are executed in the main thread.
Right good point. I think the signal's documentation should state that it has the basic thread safety, like one thread manipulating one signal is ok. Thank you. Philippe
AMDG Philippe Vaucher <philippe.vaucher <at> gmail.com> writes:
The point is, the 'threads of execution' don't 'receive the message', as the 'slot()' methods are executed in the main thread.
Right good point.
I think the signal's documentation should state that it has the basic thread safety, like one thread manipulating one signal is ok.
it isn't ok. boost/function/function_template.hpp lines 653-659 template<typename Functor> void assign_to(Functor f) { static vtable_type stored_vtable(f); if (stored_vtable.assign_to(f, functor)) vtable = &stored_vtable; else vtable = 0; } function static is not thread safe In Christ, Steven Watanabe
Steven Watanabe wrote:
AMDG
Philippe Vaucher <philippe.vaucher <at> gmail.com> writes:
The point is, the 'threads of execution' don't 'receive the message', as the 'slot()' methods are executed in the main thread. Right good point.
I think the signal's documentation should state that it has the basic thread safety, like one thread manipulating one signal is ok.
it isn't ok.
boost/function/function_template.hpp lines 653-659
template<typename Functor> void assign_to(Functor f) { static vtable_type stored_vtable(f); if (stored_vtable.assign_to(f, functor)) vtable = &stored_vtable; else vtable = 0; }
function static is not thread safe
This was not present in 1.33.1. If Boost.Function 1.33.1 is thread-safe but 1.34 is not, I would consider this as a *serious* regression. I know this could be a big change, but I strongly suggest that Boost.Function remains thread-safe in 1.34. PS: the vtable change comes from: revision 1.84, Fri Dec 30 02:31:51 2005 UTC. Tanguy
On Thu, 01 Mar 2007 19:32:33 +0000, Steven Watanabe wrote:
it isn't ok.
boost/function/function_template.hpp lines 653-659
template<typename Functor> void assign_to(Functor f) { static vtable_type stored_vtable(f); if (stored_vtable.assign_to(f, functor)) vtable = &stored_vtable; else vtable = 0; }
function static is not thread safe
You've just succeeded in scaring the hell out of me. Just how unsafe is boost::function? -braddock
Braddock Gaskill wrote:
it isn't ok.
boost/function/function_template.hpp lines 653-659
template<typename Functor> void assign_to(Functor f) { static vtable_type stored_vtable(f); if (stored_vtable.assign_to(f, functor)) vtable = &stored_vtable; else vtable = 0; }
function static is not thread safe
You've just succeeded in scaring the hell out of me. Just how unsafe is boost::function?
As far as I can see, the situation here is less problematic than with the general thread-safety problem of function-local statics. This one is local to a template member function and the object initialization is identical for concurrent calls (addresses of two static functions which only depend on the template arguments), so you cannot end up with a corrupted stored_vtable. There is however a potential race condition regarding the internal "this local static variable is initialized" state. I think with >=2 processors running that same code, one may see that flag set with the actual object not being properly initialized from its point of view (due to write reordering by the other processor). Given the relative "distance" between this initialization code and a possible access of either manager or invoker member in the vtable, I think the problem is of a rather academical nature. But the code is still not strictly correct from my understanding. The theoretical outcome is undefined behaviour for the first copy operation or invocation of each function object with a given distinct signature and assigned functor type. Regards Timmo Stange
AMDG Timmo Stange <ts <at> sepulchrum.com> writes:
Given the relative "distance" between this initialization code and a possible access of either manager or invoker member in the vtable, I think the problem is of a rather academical nature. But the code is still not strictly correct from my understanding.
I'm afraid that it is a very real problem. In pseudo-code if(!vtable_initialized) { stored_vtable_initialized = true; stored_vtable.vtable_type(f); } if(stored_vtable.assign_to(f, functor)) vtable = &stored_vtable; else vtable = 0; Even with a single CPU if the thread is interrupted immidiately after the assignment to stored_vtable_initialized, another thread can reach stored_vtable.assign_to(f, functor) before stored_vtable is initialized. In Christ, Steven Watanabe
Steven Watanabe wrote:
Given the relative "distance" between this initialization code and a possible access of either manager or invoker member in the vtable, I think the problem is of a rather academical nature. But the code is still not strictly correct from my understanding.
I'm afraid that it is a very real problem. In pseudo-code
if(!vtable_initialized) { stored_vtable_initialized = true; stored_vtable.vtable_type(f); } if(stored_vtable.assign_to(f, functor)) vtable = &stored_vtable; else vtable = 0;
Even with a single CPU if the thread is interrupted immidiately after the assignment to stored_vtable_initialized, another thread can reach stored_vtable.assign_to(f, functor) before stored_vtable is initialized.
Right, that would be a legal implementation I just did not consider. This is a little sad since it could probably be easily fixed by a compiler vendor if the standard would cover it (with a proper memory model even for the SMP case). But as it is, this nice little optimization probably has to go. Regards Timmo Stange
AMDG Timmo Stange <ts <at> sepulchrum.com> writes:
This is a little sad since it could probably be easily fixed by a compiler vendor if the standard would cover it (with a proper memory model even for the SMP case). But as it is, this nice little optimization probably has to go.
Regards
Timmo Stange
Can't vtable_type be POD and statically initialized? In Christ, Steven Watanabe
Steven Watanabe wrote:
Can't vtable_type be POD and statically initialized?
The goal is not to have distinct types for every assign_to instantiation, but distinct objects, but I think it should be possible to restructure it to use statically initialized PODs for the actual function pointers.
In Christ, Steven Watanabe
Regards Timmo Stange
Stefan Seefeld wrote:
Philippe Vaucher wrote:
Hello,
Well, the subject says it all ;) This question comes because boost::signal's documentation says it's not thread safe, but I have trouble finding exactly why it isn't.
Let's look the following example:
Each thread is connected to the same signal, then the signal is emmited, and each threads receives the message.... Why would this not be thread safe ?
You use 'thread' in a very lose way here. There is a 'thread' as seen by the OS, i.e. an execution entity processing some code, and there is are two 'thread' types ('thread1' and 'thread2') you use.
In your description above you seem to refer to those types as 'threads', not to the execution entities.
The point is, the 'threads of execution' don't 'receive the message', as the 'slot()' methods are executed in the main thread. However, as you check the value of 'finished' in other 'threads of execution', you have to protect access to it by a mutex. For example:
struct thread1 { thread1() : finished_(false) {} void operator()() { while (true) { scoped_lock lock(mutex_); if (finished_) return; } } void slot() { scoped_lock lock(mutex_); finished_ = true; }
bool fininshed_; mutex mutex_; };
HTH, Stefan
I'm not sure whether a mutex is really needed here. Because a boolean is a native type, I don't see any particular problem if one thread reads it while it's being written by another. So I think the mutex can be safely removed. What I would however do is to use "volatile bool" instead of just "bool" Tanguy
participants (6)
-
Braddock Gaskill -
Philippe Vaucher -
Stefan Seefeld -
Steven Watanabe -
Tanguy Fautre -
Timmo Stange