7

I am writing code that will spawn two thread and then wait for them to sync up using the CyclicBarrier class. Problem is that the cyclic barrier isn't working as expected and the main thread doesnt wait for the individual threads to finish. Here's how my code looks:

 class mythread extends Thread{
   CyclicBarrier barrier;
   public mythread(CyclicBarrier barrier) { 
       this.barrier = barrier;
      }

   public void run(){
            barrier.await();
       } 
 }



class MainClass{
 public void spawnAndWait(){
    CyclicBarrier barrier = new CyclicBarrier(2);
    mythread thread1 = new mythread(barrier).start();
    mythread thread2 = new mythread(barrier).start();
    System.out.println("Should wait till both threads finish executing before printing this");
  }
}

Any idea what I am doing wrong? Or is there a better way to write these barrier synchronization methods? Please help.

spender
  • 117,338
  • 33
  • 229
  • 351
Ritesh M Nayak
  • 8,001
  • 14
  • 49
  • 78

4 Answers4

14

During execution of your main thread you create two other threads and tell them to wait for each other. But you wrote nothing to make your main thread to wait for them and complain it doesn't wait. Try

CyclicBarrier barrier = new CyclicBarrier(3);
mythread thread1 = new mythread(barrier).start();
mythread thread2 = new mythread(barrier).start();
barrier.await(); // now you wait for two new threads to reach the barrier.
System.out.println("Should wait till both threads finish executing before printing this");

BTW. Don't extend Thread class unless you have to. Implement Runnable and pass implementations to Thread objects. Like this:

class MyRunnable implements Runnable {
    public void run(){
        // code to be done in thread
    }
}

Thread thread1 = new Thread(MyRunnable);
thread1.start();

EDIT
Justification for avoiding extending Thread.
The rule of thumb is as little coupling as possible. Inheritance is a very strong connection between classes. You have to inherit from Thread if you want to change some of its default behaviour (i.e. override some methods) or want to access some protected fields of class Thread. If you don't want it, you choose looser coupling - implementing Runnable and passing it as a constructor parameter to Thread instance.

Tadeusz Kopec for Ukraine
  • 12,283
  • 6
  • 56
  • 83
  • However, this one makes it synchronous while the OP seems to want a truly asynchronous approach. – Chandra Sekar Mar 26 '10 at 11:18
  • 1
    @Chandru: Well, either "Should wait till both threads finish executing before printing this" or asynchronous. You can't wait asynchronously. – Tadeusz Kopec for Ukraine Mar 26 '10 at 11:46
  • If I understand it correctly, what he needs is to print that line after the 2 threads complete their job, which can be achieved by doing it in the barrier action. This way if there are other operations in the current thread which don't depend on completion of the other 2 threads, they can continue. BTW I was not the one who down-voted you. It is still a valid solution if all other operations in current thread depended on the 2 threads' completion. – Chandra Sekar Mar 26 '10 at 11:53
  • Thank you guys. I have learn't a lot more on threads thanks to this question. A small question though. Why is extending Thread not a good option? – Ritesh M Nayak Mar 26 '10 at 13:31
2

Pass a Runnable instance to the constructor of your CyclicBarrier like this.

CyclicBarrier barrier = new CyclicBarrier(2, new Runnable() {

    @Override
    public void run() {
        System.out.println("Should wait till both threads finish executing before printing this");
    }
});

new mythread(barrier).start();
new mythread(barrier).start();
Chandra Sekar
  • 10,683
  • 3
  • 39
  • 54
  • Still doesn't wait for both the programs to finish before exiting. Is there an explicit way I can say, wait till both thread1 and thread2 finish? – Ritesh M Nayak Mar 26 '10 at 10:02
  • If you want a truly async wait for the threads' completion why bother if the method exits before their completion? Generally you'd want to do something when both the threads finish execution. Just put that in the callback to CyclicBarrier and you are good to go. – Chandra Sekar Mar 26 '10 at 10:22
2

You are looking for Thread.join() method...

thread1.join();
thread2.join();
System.out.println("Finished");

EDIT: because of the comments...

And if you don't want to wait forever you can also specify the maximum number of milliseconds plus nanoseconds to wait for the thread to die

pgras
  • 12,614
  • 4
  • 38
  • 46
  • But this is tricky because if thread1.join() doesn't happen then thread2.join() will wait infinitely. The reason I chose CyclicBarrier is because it seems truly asynchronous in its wait. – Ritesh M Nayak Mar 26 '10 at 10:08
  • 1
    @Ritesh but if thread1.join won't happen for what are you waiting then? whats your usecase? do you need an "EITHER thread1 OR thread2 finished" implementation? pgras shows how to implement 'wait until both finished' – Karussell Mar 26 '10 at 11:31
  • 1
    @Ritesh, Either you want to wait for both `thread1` and `thread2` to exit (in which case this answer is correct), or you do not (in which case the version in the question is correct.) – finnw Mar 26 '10 at 11:32
  • @finnw There is a case where you don't really want to block all the remaining operations in current thread but only some of them needs to be done after the barrier. – Chandra Sekar Mar 26 '10 at 11:57
  • My use case is that I want both thread1.join() and thread2.join to work. I tried this out and it works, what happens when thread1 throws an exception, does the join method stil work? – Ritesh M Nayak Mar 26 '10 at 13:28
  • @Ritesh yes, `thread1.join()` returns normally if `thread1` terminates due to an exception. `join()` throws only if the calling thread is interrupted. It does not (unlike `Future.get()`) propagate exceptions from the worker thread to the calling thread. – finnw Mar 26 '10 at 15:40
1

Cyclic barrier is not the correct choice in this case. You must use CountDownLatch here.

I assume you have are invoking spawnAndWait method from the main method.

The reason this wont work is that the CyclicBarrier has 2 constructors. To perform post-operations you must use a 2 parameter constructor. The most important thing to remember is that the main thread will not wait by the await method; but will continue to execute. However the Thread specified in the CyclicBarrier constructor will only run when all spawned threads stop at the barrier(by the await method)

axel22
  • 32,045
  • 9
  • 125
  • 137
roy2601
  • 11
  • 1