[thread] variables in thread examples require 'volatile'?
Dear Boosters, I'm involved in a bit of a dispute in which a colleague is claiming that 'volatile' is necessary to make certain synchronization patterns correct. As part of my response I pointed out that the boost.thread examples don't use it, but he maintains his position - saying that the examples are flawed. I'm frankly somewhat out of my element here, and I'd like to get input from people who have presumably thought about this stuff a good deal. The example in question is here, under "synopsis": http://www.boost.org/doc/libs/1_41_0/doc/html/thread/synchronization.html#th... The claim is that 'volatile' is needed on the boolean, otherwise the compiler might cache the boolean in a register, and the loop would never exit. He cites for support two articles: http://www.ddj.com/cpp/184403766 https://www.securecoding.cert.org/confluence/display/seccode/DCL17-C.+Beware... Who's right? Thanks, -Gabe
AMDG Gabriel Redner wrote:
I'm involved in a bit of a dispute in which a colleague is claiming that 'volatile' is necessary to make certain synchronization patterns correct. As part of my response I pointed out that the boost.thread examples don't use it, but he maintains his position - saying that the examples are flawed. I'm frankly somewhat out of my element here, and I'd like to get input from people who have presumably thought about this stuff a good deal.
The example in question is here, under "synopsis": http://www.boost.org/doc/libs/1_41_0/doc/html/thread/synchronization.html#th...
The claim is that 'volatile' is needed on the boolean, otherwise the compiler might cache the boolean in a register, and the loop would never exit. He cites for support two articles:
http://www.ddj.com/cpp/184403766 https://www.securecoding.cert.org/confluence/display/seccode/DCL17-C.+Beware...
Who's right?
volatile is not needed in that example because access is protected by a mutex. In Christ, Steven Watanabe
On Dec 7, 2009, at 7:56 PM, Steven Watanabe wrote:
AMDG
Gabriel Redner wrote:
I'm involved in a bit of a dispute in which a colleague is claiming that 'volatile' is necessary to make certain synchronization patterns correct. As part of my response I pointed out that the boost.thread examples don't use it, but he maintains his position - saying that the examples are flawed. I'm frankly somewhat out of my element here, and I'd like to get input from people who have presumably thought about this stuff a good deal.
The example in question is here, under "synopsis": http://www.boost.org/doc/libs/1_41_0/doc/html/thread/synchronization.html#th...
The claim is that 'volatile' is needed on the boolean, otherwise the compiler might cache the boolean in a register, and the loop would never exit. He cites for support two articles:
http://www.ddj.com/cpp/184403766 https://www.securecoding.cert.org/confluence/display/seccode/DCL17-C.+Beware...
Who's right?
volatile is not needed in that example because access is protected by a mutex.
In this example: ---- boost::condition_variable cond; boost::mutex mut; bool data_ready; void process_data(); void wait_for_data_to_process() { boost::unique_lockboost::mutex lock(mut); while(!data_ready) { cond.wait(lock); } process_data(); } ---- What prevents the compiler from leaving data_ready in a register while in the while loop, hence never seeing some other thread setting it? The mutex is released while waiting on the condition, so data_ready is not protected while in cond.wait(). - Stoney -- Stonewall Ballard stoney@sb.org http://stoney.sb.org/
On Mon, Dec 7, 2009 at 8:16 PM, Stonewall Ballard
On Dec 7, 2009, at 7:56 PM, Steven Watanabe wrote:
AMDG
Gabriel Redner wrote:
I'm involved in a bit of a dispute in which a colleague is claiming that 'volatile' is necessary to make certain synchronization patterns correct. As part of my response I pointed out that the boost.thread examples don't use it, but he maintains his position - saying that the examples are flawed. I'm frankly somewhat out of my element here, and I'd like to get input from people who have presumably thought about this stuff a good deal.
The example in question is here, under "synopsis": http://www.boost.org/doc/libs/1_41_0/doc/html/thread/synchronization.html#th...
The claim is that 'volatile' is needed on the boolean, otherwise the compiler might cache the boolean in a register, and the loop would never exit. He cites for support two articles:
http://www.ddj.com/cpp/184403766 https://www.securecoding.cert.org/confluence/display/seccode/DCL17-C.+Beware...
Who's right?
volatile is not needed in that example because access is protected by a mutex.
In this example: ---- boost::condition_variable cond; boost::mutex mut; bool data_ready;
void process_data();
void wait_for_data_to_process() { boost::unique_lockboost::mutex lock(mut); while(!data_ready) { cond.wait(lock); } process_data(); } ----
What prevents the compiler from leaving data_ready in a register while in the while loop, hence never seeing some other thread setting it? The mutex is released while waiting on the condition, so data_ready is not protected while in cond.wait().
I might agree. Although I doubt volatile is the best thing here either. Volatile may prevent it from being stored in registers, but it will not prevent it from being cached. data_ready will eventually be ready when the cache's finally sync, but you should be using a condition variable instead of a global bool if you want it to happen more along the lines of 'now'. It is possible that the cond.wait may introduce a memory barrier that forces the cache among multiple CPU's to sync up though, meaning that the code could actually be correct. Hmm, now that I think of it, I think that example is correct. I see no problem then.
On Mon, Dec 7, 2009 at 10:35 PM, OvermindDL1
On Mon, Dec 7, 2009 at 8:16 PM, Stonewall Ballard
wrote: On Dec 7, 2009, at 7:56 PM, Steven Watanabe wrote:
AMDG
Gabriel Redner wrote:
volatile is not needed in that example because access is protected by a mutex.
In this example: ---- boost::condition_variable cond; boost::mutex mut; bool data_ready;
void process_data();
void wait_for_data_to_process() { boost::unique_lockboost::mutex lock(mut); while(!data_ready) { cond.wait(lock); } process_data(); } ----
What prevents the compiler from leaving data_ready in a register while in the while loop, hence never seeing some other thread setting it? The mutex is released while waiting on the condition, so data_ready is not protected while in cond.wait().
I might agree. Although I doubt volatile is the best thing here either. Volatile may prevent it from being stored in registers, but it will not prevent it from being cached. data_ready will eventually be ready when the cache's finally sync, but you should be using a condition variable instead of a global bool if you want it to happen more along the lines of 'now'. It is possible that the cond.wait may introduce a memory barrier that forces the cache among multiple CPU's to sync up though, meaning that the code could actually be correct. Hmm, now that I think of it, I think that example is correct. I see no problem then.
Volatile and threading/barriers are 2 different things, which only appear related. volatile is for dealing with the compiler, barriers are for dealing with the CPU(s). The reason that data_ready is NOT 'left' in a register is because a function with unknown side-effects (cond.wait) is being called. You could just as easily have: while (!data_ready) some_library_function(); and the compiler will ensure that data_ready is re-read (ie not left in a register) because it is a global, and the compiler is unsure whether or not some_library_function() modifies data_ready or not. In theory the compiler could go look into some_library_function() and figure that out, but in practice it doesn't. As for memory barriers, cond.wait() locks and unlocks the mutex, which puts in the necessary memory barriers. Note that the order of lock/unlock/relock is such that data_ready is only read while holding the lock. Of course, nothing here says whether the other thread *wrote* data_ready inside a lock or with the necessary release-barrier, but let's assume that it did. So recap:
volatile is not needed in that example because access is protected by a mutex.
No, I'd say volatile is not needed because functions with unknown side-effects are being called (and/or if the mutex code was somehow magically inlined, then we can assume the compiler recognizes the memory-barrier intrinsics and forces memory re-reads because of that). And
It is possible that the cond.wait may introduce a memory barrier that forces the cache among multiple CPU's to sync up
- cond.wait DOES introduce a memory barrier. Probably 2 - a release on unlock(mutex) and an acquire on lock(mutex) - it is not really 'cache syncing' that is the problem (most CPUs have cache-coherency guarantees) it is the relative ordering of reads and writes (and RE-ordering by the CPU / memory bus) that causes the 'visibility' problems. Tony
On Dec 8, 2009, at 1:21 AM, Gottlob Frege wrote:
... The reason that data_ready is NOT 'left' in a register is because a function with unknown side-effects (cond.wait) is being called. You could just as easily have:
while (!data_ready) some_library_function();
and the compiler will ensure that data_ready is re-read (ie not left in a register) because it is a global, and the compiler is unsure whether or not some_library_function() modifies data_ready or not. In theory the compiler could go look into some_library_function() and figure that out, but in practice it doesn't.
As for memory barriers, cond.wait() locks and unlocks the mutex, which puts in the necessary memory barriers. Note that the order of lock/unlock/relock is such that data_ready is only read while holding the lock. Of course, nothing here says whether the other thread *wrote* data_ready inside a lock or with the necessary release-barrier, but let's assume that it did.
So recap:
volatile is not needed in that example because access is protected by a mutex.
No, I'd say volatile is not needed because functions with unknown side-effects are being called (and/or if the mutex code was somehow magically inlined, then we can assume the compiler recognizes the memory-barrier intrinsics and forces memory re-reads because of that).
I woke up in the middle of the night last night with that realization, so I completely agree with this analysis. Don't you hate it when that happens? :-)
...
- Stoney -- Stonewall Ballard stoney@sb.org http://stoney.sb.org/
Volatile and threading/barriers are 2 different things, which only appear related. volatile is for dealing with the compiler, barriers are for dealing with the CPU(s).
The reason that data_ready is NOT 'left' in a register is because a function with unknown side-effects (cond.wait) is being called. You could just as easily have:
while (!data_ready) some_library_function();
and the compiler will ensure that data_ready is re-read (ie not left in a register) because it is a global, and the compiler is unsure whether or not some_library_function() modifies data_ready or not. In theory the compiler could go look into some_library_function() and figure that out, but in practice it doesn't.
As for memory barriers, cond.wait() locks and unlocks the mutex, which puts in the necessary memory barriers. Note that the order of lock/unlock/relock is such that data_ready is only read while holding the lock. Of course, nothing here says whether the other thread *wrote* data_ready inside a lock or with the necessary release-barrier, but let's assume that it did.
So recap:
volatile is not needed in that example because access is protected by a mutex.
No, I'd say volatile is not needed because functions with unknown side-effects are being called (and/or if the mutex code was somehow magically inlined, then we can assume the compiler recognizes the memory-barrier intrinsics and forces memory re-reads because of that).
And
It is possible that the cond.wait may introduce a memory barrier that forces the cache among multiple CPU's to sync up
- cond.wait DOES introduce a memory barrier. Probably 2 - a release on unlock(mutex) and an acquire on lock(mutex) - it is not really 'cache syncing' that is the problem (most CPUs have cache-coherency guarantees) it is the relative ordering of reads and writes (and RE-ordering by the CPU / memory bus) that causes the 'visibility' problems.
Thank you for the very clear and concise explanation! -Gabe
On Mon, Dec 7, 2009 at 7:16 PM, Stonewall Ballard
On Dec 7, 2009, at 7:56 PM, Steven Watanabe wrote:
AMDG
Gabriel Redner wrote:
I'm involved in a bit of a dispute in which a colleague is claiming that 'volatile' is necessary to make certain synchronization patterns correct. As part of my response I pointed out that the boost.thread examples don't use it, but he maintains his position - saying that the examples are flawed. I'm frankly somewhat out of my element here, and I'd like to get input from people who have presumably thought about this stuff a good deal.
The example in question is here, under "synopsis": http://www.boost.org/doc/libs/1_41_0/doc/html/thread/synchronization.html#th...
The claim is that 'volatile' is needed on the boolean, otherwise the compiler might cache the boolean in a register, and the loop would never exit. He cites for support two articles:
http://www.ddj.com/cpp/184403766 https://www.securecoding.cert.org/confluence/display/seccode/DCL17-C.+Beware...
Who's right?
volatile is not needed in that example because access is protected by a mutex.
In this example: ---- boost::condition_variable cond; boost::mutex mut; bool data_ready;
void process_data();
void wait_for_data_to_process() { boost::unique_lockboost::mutex lock(mut); while(!data_ready) { cond.wait(lock); } process_data(); } ----
What prevents the compiler from leaving data_ready in a register while in the while loop, hence never seeing some other thread setting it? The mutex is released while waiting on the condition, so data_ready is not protected while in cond.wait().
There are a few things it could be doing so that it could look like that and still be correct: a) An explicit barrier intrinsic, which is a more optimal way to get volatile semantics. (ie. VC++'s _ReadWriteBarrier) b) An atomic intrinsic which the compiler recognizes as an implicit barrier. (ie. VC++'s _InterlockedCompareExchange) c) A function with unknown side effects is being called somewhere. (ie. anything the compiler doesn't have a full definition of... like an operating system's own mutex functions) -- Cory Nelson http://int64.org
2009/12/7 Gabriel Redner
Dear Boosters,
I'm involved in a bit of a dispute in which a colleague is claiming that 'volatile' is necessary to make certain synchronization patterns correct. As part of my response I pointed out that the boost.thread examples don't use it, but he maintains his position - saying that the examples are flawed. I'm frankly somewhat out of my element here, and I'd like to get input from people who have presumably thought about this stuff a good deal.
The example in question is here, under "synopsis":
http://www.boost.org/doc/libs/1_41_0/doc/html/thread/synchronization.html#th...
The claim is that 'volatile' is needed on the boolean, otherwise the compiler might cache the boolean in a register, and the loop would never exit. He cites for support two articles:
http://www.ddj.com/cpp/184403766
https://www.securecoding.cert.org/confluence/display/seccode/DCL17-C.+Beware...
Who's right?
Thanks, -Gabe _______________________________________________ Boost-users mailing list Boost-users@lists.boost.org http://lists.boost.org/mailman/listinfo.cgi/boost-users
Required reading http://software.intel.com/en-us/blogs/2007/11/30/volatile-almost-useless-for...
participants (7)
-
Cory Nelson
-
Gabriel Redner
-
Gottlob Frege
-
liam mail
-
OvermindDL1
-
Steven Watanabe
-
Stonewall Ballard