views:

61

answers:

5

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?

+2  A: 

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.)

Tom Hawtin - tackline
+1 good point for adding some perspective to looking at more than just the boolean variable.
Romain Hippeau
Your point is well made but there is no context, based on the question asked, for introducing the composite argument. The question was about a boolean reader/writer problem. Showboating?
bjg
@bjg Threading involves context. It's inherently non-local. I believe it is likely that the problem in hand goes further than one boolean. Certainly it would be unhelpful for someone new to the area to solve a special case problem with tricky code.
Tom Hawtin - tackline
A: 

I would suggest that the boolean be declared volatile and that read AND write accesses be synchronized

bjg
That sounds confused. (Also `volatile` is surprising unhelpful (excapt at a low-level) as operations generally require multiple component operations.)
Tom Hawtin - tackline
@tackline Do you mean it's confused because it's a belt and braces approach? If so, then while I understand the Java programming model may suggest that volatile on its own is sufficient I would still be suspicious of this claim with respect to all JVM implementations I might encounter especially when running on muti-core environments. In any event - even if my paranoia is unfounced - volatile on its own is usually not a strong enough hint to maintainers that there is something important that they need to be aware of in relation to the boolean in question
bjg
@bjg. You should either declare a variable `volatile` or use `synchronized` accessors. Doing both just hinders performance and adds complexity. And Tom is right that the correct usage of volatile is mostly reserved by java.concurrent gurus.
Alexander Pogrebnyak
+3  A: 

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:

  1. The value of this variable will never be cached thread-locally: all reads and writes will go straight to "main memory";
  2. 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.

Romain Hippeau
`volatile` is precisely specified in the JLS and JVM spec. However, it does appear to be used far more often than the consequences are understood.
Tom Hawtin - tackline
@Tom Hawtin - One needs to understand the limitations. It is a powerful tool in a limited set of situations.
Romain Hippeau
While what you said is true of boolean (and in some cases int) primitives, be careful with int when performing operations like int++. While this appears to be a single operation, it's not really and just using volatile in a case like this is not enough.
Jeff Storey
@Jeff Storey int++ is actually read variable, increment and write, so is +=
Romain Hippeau
@Romain, yes my point exactly. It's not a single operation.
Jeff Storey
A: 

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.

Alexander Pogrebnyak
Care to explain the downvote?
Alexander Pogrebnyak
A: 

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

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

nos