[thread] possible raise in future?

Hello Vicente, after analyzing the code of future<> from boost.thread I think the code might contain raise condition. future.hpp:226: future_object_base defined 'bool done'. future_object_base::mark_finished_internal() sets 'done' tot true (line 308) and future_object_base::wait_internal() reads it (line 347). Shouldn't 'done' be of type atomic<bool >? I believe that the compiler can reorder the usage of 'done' and two threads (writer thread calling make_finished_internal() and reader calling wait_internal()) might see different values. At least I think it might be possible that some thread will always be waiting in waiters.wait() (line 349), because the write r thread sets done=true and call waiters.notify_all() and a reader thread calls later wait_internal() but does not see that done is true because of memory reordering etc. and will be blocked in waiters.wait()forever. What do you think? best regards, Oliver

Le 03/02/13 13:04, Oliver Kowalke a écrit :
Hello Vicente,
after analyzing the code of future<> from boost.thread I think the code might contain raise condition. future.hpp:226: future_object_base defined 'bool done'. future_object_base::mark_finished_internal() sets 'done' tot true (line 308) and future_object_base::wait_internal() reads it (line 347).
Hi, both uses are protected with boost::unique_lock<boost::mutex> &lock. Am I missing something? Best, Vicente
Shouldn't 'done' be of type atomic<bool >? I believe that the compiler can reorder the usage of 'done' and two threads (writer thread calling make_finished_internal() and reader calling wait_internal()) might see different values. At least I think it might be possible that some thread will always be waiting in waiters.wait() (line 349), because the write r thread sets done=true and call waiters.notify_all() and a reader thread calls later wait_internal() but does not see that done is true because of memory reordering etc. and will be blocked in waiters.wait()forever.
What do you think?
best regards, Oliver
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

both uses are protected with boost::unique_lock<boost::mutex> &lock. Am I missing something?
Best, Vicente
Oh - I see you have already answered. I'm not shure if reordering is still possible. Var done is not volatile and because the implementation is in the header it might be inlined. I don't know if the lock creates something like a memory barrier (happend before etc.). I'm using your future implementation - I replaced mutex and condition with my own implementation (using atomics). Unit-test which use future do sometimes not return. If I use condition::wait() instead of future<>::wait() the code works perfect. Of curse it might be possible that my impl is buggy (but I use simple spinlock). What I want to say - I'm uncertain. best, Oliver

Le 03/02/13 17:21, Oliver Kowalke a écrit :
both uses are protected with boost::unique_lock<boost::mutex> &lock. Am I missing something? Best, Vicente Oh - I see you have already answered.
I'm not shure if reordering is still possible. Var done is not volatile and because the implementation is in the header it might be inlined. I don't know if the lock creates something like a memory barrier (happend before etc.). Well, a lock is a lock. Its purpose is to protect from race conditions, so I don't see where the problem could come from.
I'm using your future implementation - I replaced mutex and condition with my own implementation (using atomics). Unit-test which use future do sometimes not return. If I use condition::wait() instead of future<>::wait() the code works perfect. Of curse it might be possible that my impl is buggy (but I use simple spinlock). What I want to say - I'm uncertain.
I suspect that I could not help you there. Best, Vicente
participants (2)
-
Oliver Kowalke
-
Vicente J. Botet Escriba