rents rents - 13 days ago 8
Java Question

Is this an incorrect use of intrinsic condition queue in java?

This program attempts to print numbers 1 to 10 in a sequential manner, 1 thread prints odd numbers and the second threads prints even numbers.

I have been reading JCIP book and it says:


Ensure that the state variables making up the condition predicate are guarded by the lock associated with the condition queue.


In the below program, the condition queue will correspond to static member 'obj1' while the state variable that makes up the condition predicate is static volatile member 'count'. (let me know if I am wrong in the interpretation of condition, state variable, condition predicate)

The below program works correctly but is clearly violating the above idiom. Have I understood what the author is trying to say correctly? Is the below code really a poor programming practice (which happens to work correctly)

Can you give me an example where not following the above idiom will make me run into problems?

public class OddEvenSynchronized implements Runnable {
static Object obj1 = new Object(); // monitor to share data
static volatile int count =1; // condition predicate
boolean isEven;
public OddEvenSynchronized(boolean isEven) { //constructor
this.isEven=isEven;
}
public void run (){
while (count<=10){
if (this.isEven == true){
printEven(); //print an even number
}
else{
printOdd(); //print an odd number
}
}
}

public static void main(String[] args) {
Thread t1 = new Thread (new OddEvenSynchronized(true));
Thread t2 = new Thread (new OddEvenSynchronized(false));
//start the 2 threads
t1.start();
t2.start();
}

void printEven(){
synchronized (obj1) {
while (count%2 != 0){
try{
obj1.wait();
}catch (InterruptedException e) {
e.printStackTrace();
}
}
}
System.out.println("Even"+count);

count++; //unguarded increment (violation)
synchronized (obj1) {
obj1.notifyAll();
}
} //end method
void printOdd(){
synchronized (obj1) {
while (count%2 == 0){
try{
obj1.wait();
}catch (InterruptedException e) {
e.printStackTrace();
}
}
}
System.out.println("Odd"+count);
count++; //unguarded increment (violation)
synchronized (obj1) {
obj1.notifyAll();
}

} //end method
} //end class

Answer

The only reason I have been able to come up with for this question, as hinted by my discussion with John Kugelman in his answer (please correct if something is wrong):

1st Key insight: In Java, there is only one condition queue associated with the object's monitor. And although they share the condition queue, there condition predicates are different. This sharing results in unnecessary wakeup ->check condition predicate -> sleep again. So although inefficient, they will still behave as kind of separate condition queues if coded properly ( while ( condition predicate ) { thread.wait() } )

In the above program, the condition predicates

  1. count%2 == 0
  2. count%2 != 0

are different, although they are part of the same condition queue (i.e. doing a notify() on this object's monitor will wake both of them, however only one would be able to proceed at a time).

The 2nd key insight: The volatile count variable ensure memory visibility.

Conclusion:

As soon as we introduce another thread with the same condition predicate, the program will be susceptible to race conditions (if not other defects).

Also, note that usually wait() notify() mechanism is employed for object with same condition predicated, for example, waiting for a resource lock. The above program is usually used in interviews and I doubt if it would be common in real-life code.

So, if there are two or more threads in the same condition queue with different condition predicates, and the condition predicate variable is volatile (and hence ensures memory visibility), then ignoring the above advice can produce a correct program. Although this is of little significance, this really helped me understand multi-threading better.