Ankit_ceo2
Ankit_ceo2

Reputation: 317

The threads are not printing in a sequence in my program

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

Answers (1)

Jorn Vernee
Jorn Vernee

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

Related Questions