Reputation: 317
I have the program below to sequence the 3 threads. For example first the first thread should print followed by second and then third thread. Bu the program below is not actually doing that and printing some random sequence. I have seen some programs on stackoverflow itself which execute and try to print in sequence.
But I am really trying hard to understand why the program below is not working and what is the thing I am not able to understand.
package my.tutorial.java;
import java.util.concurrent.Executor;
import java.util.concurrent.Executors;
public class SequenceThreads {
private static final Object lock = new Object();
static class Task implements Runnable {
private final String tName;
private final int turnId;
private static int nextTurn = 1;
public Task(String tName, int turnId) {
this.tName = tName;
this.turnId = turnId;
}
@Override
public void run() {
while (true) {
synchronized (lock) {
if (nextTurn != turnId) {
try {
// System.out.println(tName + " went waiting " + nCount);
lock.wait();
} catch (InterruptedException e) {
e.printStackTrace();
}
}
// System.out.println(tName + " went waiting");
System.out.println(tName + " Executed ");
++nextTurn;
if (nextTurn == 4)
nextTurn = 1;
// System.out.println(nextTurn);
try {
Thread.sleep(1000);
} catch (InterruptedException e) {
e.printStackTrace();
}
lock.notify();
}
}
}
}
public static void main(String[] args) throws InterruptedException {
final Executor executor = Executors.newFixedThreadPool(3);
//AtomicInteger nCount = new AtomicInteger(1);
final Task task1 = new Task("T1", 1);
final Task task2 = new Task("T2", 2);
final Task task3 = new Task("T3", 3);
executor.execute(task1);
executor.execute(task2);
executor.execute(task3);
}
}
The expected result should be
T1 Executed
T2 Executed
T3 Executed
T1 Executed
T2 Executed
T3 Executed
But the actual result is
T1 Executed
T2 Executed
T1 Executed
T1 Executed
T3 Executed
T3 Executed
T1 Executed
T3 Executed
T3 Executed
Upvotes: 1
Views: 736
Reputation: 33865
Your code has 2 problems.
1.) You check if it's a thread's turn to print with the nextTurn != turnId
in an if
statement. That means that if a thread reaches this if
, it's not the thread's turn, it waits once, can then be woken up again, it might still not be that thread's turn, but it doesn't check again and just continues executing out of order.
To fix that, turn this if
into a while
.
2.) notify()
does not wake up all waiting threads. That means that you might get a situation where the wrong thread is woken up, check's and sees that it's not the thread's turn yet, goes to sleep again, and then the notify()
call is never reached, so the right thread is never woken up. In that case we are dealocked, unless a spurious wakeup occurs.
For that you can use notifyAll()
instead.
After fixing those 2 things I'm seeing the expected output:
T1 Executed
T2 Executed
T3 Executed
T1 Executed
T2 Executed
T3 Executed
T1 Executed
T2 Executed
T3 Executed
...
Upvotes: 3