0

I would like for thread A to iterate once through the for loop, wait for thread B to loop once through the for loop and wait for thread C to loop once through the for loop. And then all over again. I tried to use wait() and notify() but I got nowhere. How could one solve the problem?

public class Test extends Thread {
    static int counter=0;

    synchronized public void run() {
        for(int i=0; i<4; i++) {
            System.out.println(Thread.currentThread().getName()+" "+counter++);
        }
    }


    public static void main(String[] args) throws InterruptedException {
        Test t1 = new Test();
        Test t2 = new Test();
        Test t3 = new Test();

        t1.setName("A");
        t2.setName("B");
        t3.setName("C");

        t1.start();
        t2.start();
        t3.start();

    }
}

It should print the following to the console:

A 0
B 1
C 2
A 3
B 4
C 5
A 6
B 7
C 8
A 9
B 10
C 11

But using the code I get a random output like this:

A 0
A 3
A 4
A 5
C 2
C 6
C 7
B 1
C 8
B 9
B 10
B 11
Tobias Mai
  • 111
  • 2
  • 6
  • 2
    You're synchronizing on `this`, and there are 3 of Test `this`'s present -- meaning you're not synchronizing at all. But even using the same lock won't produce the output that you desire. – Hovercraft Full Of Eels Jan 26 '16 at 15:31
  • Also it is generally accepted that it is better to implement Runnable, rather than extend Thread. http://stackoverflow.com/questions/541487/implements-runnable-vs-extends-thread – Will Jan 26 '16 at 15:34
  • 2
    The problem is that even if you synchronized on the same instance (called a monitor in the docs) there's no guarantee on the order of threads. You'd need some other mechanism but the main question would be: if the threads should run sequentially anyways then why use threads? – Thomas Jan 26 '16 at 15:37

4 Answers4

0

Since you are making instance method syncronized it will try to obtain monitor of current instance and as all 3 threads are different instance all obtain and release monitor independently. You should probably consider using join() to wait for other threads.

You can do something like -

public class Test extends Thread {
    static int counter=0;
    Test lockTest;

    public Test(){}
    public Test(Test t) {
        this.lockTest = t;
    }


    public void run() {
        for(int i=0; i<4; i++) {
            System.out.println(Thread.currentThread().getName()+" "+counter++);
        }
        if(lockTest != null)
            lockTest.join();   //wait for other thread
    }

    public static void main(String[] args) throws InterruptedException {
       Test t1 = new Test(null);
       Test t2 = new Test(t1);
       Test t3 = new Test(t2);

       t1.setName("A");
       t2.setName("B");
       t3.setName("C");

       t1.start();
       t2.start();
       t3.start();

    }

}
Aniket Thakur
  • 66,731
  • 38
  • 279
  • 289
0

If you really need to solve it that way, you can use something like code below. But as other people mentioned it doesn't really make much sense to use threads in such case. I would suggest looking into Future class and ExecutorService.submit if you need to wait for results of computations from various tasks (or even better, guava ListenableFuture). Possibly CyclicBarrier, CountDownLatch, some kind of blocking queue... hard to say without knowing your real usecase, rather than mysterious code question.

public class Test extends Thread {
static int counter=0;

private Test previous;

public void run() {
    for(int i=0; i<4; i++) {
        synchronized(previous) {
            try {
                previous.wait();
            } catch (InterruptedException e) {
                e.printStackTrace();
            }
        }
        System.out.println(Thread.currentThread().getName()+" "+counter++);
        synchronized(this) {
            this.notify();
        }
    }
}


public static void main(String[] args) throws InterruptedException {
    Test t1 = new Test();
    Test t2 = new Test();
    Test t3 = new Test();

    t1.setName("A");
    t2.setName("B");
    t3.setName("C");

    t1.previous = t3;
    t2.previous = t1;
    t3.previous = t2;

    t1.start();
    t2.start();
    t3.start();

    synchronized(t3) {
        t3.notify();
    }

}
}
Artur Biesiadowski
  • 3,595
  • 2
  • 13
  • 18
0

If you want to loop endlessly between three Runnables in sequence, create the Runnables and feed them into a single threaded ThreadPoolExecutor or ExecutorService like so:

while (running)
{
    ExecutorService threadPool = Executors.newSingleThreadExecutor(Executors.defaultThreadFactory());
    threadPool.execute(new RunnableA());
    threadPool.execute(new RunnableB());
    threadPool.execute(new RunnableC());
    threadPool.shutdown();
    threadPool.awaitTermination();//put in a try/catch, omitted for brevity
}

That's what you really want. Note that the ExecutorService above has an unbounded queue so you'll have to limit the rate at which you submit tasks or use something like a ThreadPoolExecutor that lets you use bounded queues. Or you could just create a fresh ExecutorService with each loop, call shutdown() after each loop and then await termination at the end of the loop.

In case you were unaware of this, each of the RunnableA/B/C classes above should be soemthing like this:

public class RunnableA implements Runnable()
{
    public void run()
    {
        //do stuff, ie, your loop
    }
}

Of course there is always the option to just put all three loops in one method body, which has the advantage of being simpler.

public void doStuff()
{
    //loopA
    //loopB
    //loopC
}
Jim W
  • 492
  • 2
  • 11
-1

You want a fix type of output i.e. A X, B X, C X, A X, ... . And, you cannot fix the order of an execution of a threads.
And, the actual output you get is based on your syncronized keyword. Which fixes that first A will be printed then B and so on.
You cannot predict the execution of order of threads.

Ashish Ani
  • 324
  • 1
  • 7
  • Probably because he asked _how_ to order thread execution, not to reassert him that they way he does it won't work reliably. And there are ways to do that, as you can see in other answers. Plus your comment about 'fixing' done by synchronized keyword is wrong, synchronized doesn't do anything in his example code. – Artur Biesiadowski Jan 27 '16 at 13:26