Hey all, I'm working on some code I inherited, it looks like one thread is setting a boolean member variable while another thread is in a while loop checking it. Will this actually work OK or should I change it to use synchronized getters or setters on the boolean var?
Almost certainly you will need to add locking at a higher level. Just adding synchronized
around single accesses to fields rarely helps. Composite operations will not be thread-safe if component operations are independently thread-safe.
As an example consider deleting the contents of a thread-safe document. The document provides two relevant thread-safe operations: deleting contents between two indexes and a length operation. So you get the length and delete from zero to the length, right? Well, there's a race as the document may change length between reading the length and deleting the contents. There is no way to make the operation thread-safe given the operations. (Example taken from Swing Text.)
I would suggest that the boolean be declared volatile and that read AND write accesses be synchronized
In the case of reading and writing a primitive like bool or int declaring them as volatile will be plenty. When one threads read the other thread would have finished writing. The variable will never be in an invalid state.
It's probably fair to say that on the whole, the volatile keyword in Java is poorly documented, poorly understood, and rarely used. To make matters worse, its formal definition actually changed as of Java 5. Essentially, volatile is used to indicate that a variable's value will be modified by different threads.
Declaring a volatile Java variable means:
- The value of this variable will never be cached thread-locally: all reads and writes will go straight to "main memory";
- Access to the variable acts as though it is enclosed in a synchronized block, synchronized on itself.
We say "acts as though" in the second point, because to the programmer at least (and probably in most JVM implementations) there is no actual lock object involved.
If you use java 1.5+, You should use Condition synchronization primitive.
If you follow the link it has a nice example on how to use it.
It's not guaranteed to work if it's a plain boolean, one of the threads might not see the updated boolean due to the memory model of java , you can
- create a synchronized getter/setter or
- declare the boolean as volatile, http://www.ibm.com/developerworks/java/library/j-jtp06197.html is a good resource for understaning volatile.
Keep in mind that none of these might "work" if the thread reading this boolean depends on the previous value of the boolean - it might miss changes to that boolean, e.g.
Thread1:
while(foo.myVolatileBoolean) {
...
}
Thread2:
foo.myVolatileBoolean = false; //thread 1 might never catch thisone.
...
foo.myVolatileBoolean = true;
If this is an issue and you need to monitor changes to the boolean, consider using wait()/notify() or Condition