22

I have a thread that updates it's state from time to time and I want a second thread to be able to wait for the first thread to be done. Something like this:

Thread 1:
    while(true) {
        ...do something...
        foo.notifyAll()
        ...wait for some condition that might never happen...
        ...
    }

Thread 2:
    ...
    foo.wait();
    ...

Now this looks nice and all unless Thread 1's notifyAll() runs before Thread 2's wait(), in which case Thread 2 waits until Thread 1 notifies again (which might never happen).

My possible solutions:

a) I could use a CountDownLatch or a Future, but both have the problem that they inherently only run once. That is, in Thread 1's while loop, I would need to create a new foo to wait for each time and Thread 2 would need to ask which foo to wait for. I have a bad feeling about simply writing

while(true) {
   foo = new FutureTask(); 
   ...
   foo.set(...);
   ...wait for a condition that might never be set...
   ...
}

as I fear that at foo = new FutureTask(), what happens when someone waited for the old foo (for "some reason", set was not called, e.g. a bug in the exception handling)?

b) Or I could use a semaphore:

class Event {
   Semaphore sem;
   Event() { sem = new Semaphore(1); sem . }
   void signal() { sem.release(); }
   void reset() { sem.acquire(1); }
   void wait() { if (sem.tryAcquire(1)) { sem.release(); } }
}

But I fear that there is some race condition, if multiple threads are wait()ing for it while another one signal()s and reset()s.

Question:

Is there nothing in the Java API that resembles the Windows Event behaviour? Or, if you despise Windows, something like golang's WaitGroup (i.e. a CountDownLatch that allows countUp())? Anything?

How to do it manually:

Thread 2 cannot simply wait because of spurious wakeup and in Java there is no way to know why Object.wait() returned. So I need a condition variable that stores whether the event is signalled or not. Thread 2:

synchronized(foo) {
    while(!condition) {
        foo.wait();
    }
}

And Thread 1 of course sets condition to true in a synchronized block. Thanks to weekens for the hint!

Is there an existing class that wraps that behaviour?

Or do I need to copy and paste the code all over?

Kosta
  • 812
  • 1
  • 7
  • 13
  • What problem are you actually trying to solve? While I'm sure this problem can be resolved, I'd still like to ask so we know this is the right solution for your problem. This kind of messing about with threads and semaphores while well understood is faily prone to failures due to small mistakes so there might be a more readable, easy to understand way to do what you want to achieve. In particular it sounds like a simple evenlistener pattern where listeners register with the source of the events would be more suitable. – Vala Dec 19 '11 at 13:54
  • Yes, an event listener would solve the problem by letting thread 1 "push" updates. This is usually the preferred method, e.g. in a server environment. However, I ran into the problem of thread-safe "pulling" for updates from time to time and I never had a clean solution until now. – Kosta Dec 19 '11 at 16:29

5 Answers5

25

It is standard practice to change some state when performing notifyAll and to check some state when performing wait().

e.g.

boolean ready = false;

// thread 1
synchronized(lock) {
    ready = true;
    lock.notifyAll();
}


// thread 2
synchronized(lock) {
    while(!ready) 
        lock.wait();
}

With this approach, it doesn't matter if thread 1 or thread 2 acquires the lock first.

Some coding analysis tools will give you a warning if you use notify or wait without setting a value or checking a value.

Peter Lawrey
  • 525,659
  • 79
  • 751
  • 1,130
  • 4
    Doesn't this lead to a deadlock situation where Thread 2 grabs `lock` and won't release until it leaves the block (but it won't leave, because it's waiting for `ready` to be true, which will never happen while Thread 2 holds `lock`)? – Vala Dec 19 '11 at 16:51
  • 7
    `lock.wait()` releases the `lock` and re-acquires it before returning. – Peter Lawrey Dec 19 '11 at 18:06
  • The while loop is probably not a good idea in this version. A plain if would be enough. I've extracted the logic for this into a reusable class here - https://codingcraftsman.wordpress.com/2015/07/27/signalling-between-threads-in-java/ – Ashley Frieze Jul 27 '15 at 10:15
  • 4
    @AshleyFrieze a plain `if` is not enough in all cases. I fixed a concurrency bug last week where a loop had not been used and `wait()` was waking spuriously as it is documented it can do. – Peter Lawrey Jul 27 '15 at 11:39
  • 2
    @AshleyFrieze the problem is that `wait()` will work just fine 99.99% of the time, but under load we have seen this 0.01% cause a small percentage of tests to fail randomly in a way which is hard to reproduce. Since adding the loop the problem hasn't re-occurred after a week of testing. – Peter Lawrey Jul 27 '15 at 12:30
  • 1
    @PeterLawrey I double checked the documentation and have updated my example accordingly. Thanks for the heads up. Interestingly, there's a difficulty in achieving wait(TIMEOUT) with this method, since you can't put that in a loop. I suspect if you're happy to handle a timeout, you probably don't mind early termination. – Ashley Frieze Jul 28 '15 at 10:15
  • 1
    @AshleyFrieze agreed, if you need to use a fixed timeout, you have to get the start time and keep decreasing the timeout. – Peter Lawrey Jul 28 '15 at 10:38
3

You could use a wait() with timeout, in which case you are not risking to wait forever. Also note that wait() may return even if there was no notify() at all, so, you'll need to wrap your wait inside some conditioned loop. That's the standard way of waiting in Java.

synchronized(syncObject) {
    while(condition.isTrue()) {
        syncObject.wait(WAIT_TIMEOUT);
    }
}

(in your Thread 2)

Edit: Moved synchronized outside the loop.

weekens
  • 8,064
  • 6
  • 45
  • 62
  • The problem is that Thread 2 misses the first notifyAll(), a timeout does not help against that. But: I missed the spurious wakeup in my question, so I need to account for this too. The condition variable is actually what I need - but I prefer while(condition.isFalse()) – Kosta Dec 19 '11 at 14:01
  • Yes, you may use isFalse(), of course. Also the while loop could be inside the synchronized section. This code snippet is just to show the general idea. – weekens Dec 19 '11 at 14:20
  • The problem with this is that the condition can become true after it is checked but before the lock is acquired. This means you end up wait()ing for no reason. – Peter Lawrey Dec 19 '11 at 18:08
  • Okay, you're right. I've edited my answer to make things more correct. – weekens Dec 21 '11 at 17:53
2

The simplest way is just to say

firstThread.join();

This will be blocking until the first thread is terminated.

But you can implement the same using wait/notify. Unfortunately you have not posted your real code fragments but I guess that if wait does not exit when you call notify it happens because you did not put both into synchronized block. Pay attention that the "argument" of synchronized block must be the same for wait/notify pair.

AlexR
  • 114,158
  • 16
  • 130
  • 208
  • 1
    Sorry, but in this case, the first thread runs in a while loop indefinitely, so firstThread.join() just runs forever. wait() does not exit because it is called after notifyAll(). The synchronized blocks and wait()/notifyAll() all have the same arguments. – Kosta Dec 19 '11 at 13:57
2

I'd use a BlockingQueue between the two threads. Using wait and notify is so 5 minutes ago ;)

enum Event {
  Event,
  Stop;
}

BlockingQueue<Event> queue = new LinkedBlockingQueue<Event>();

// Thread 1
try {
  while(true) {
    ...do something...
    queue.put(Event.Event);
    ...wait for some condition that might never happen...
    ...
  }
} finally {
  // Tell other thread we've finished.
  queue.put(Event.Stop};
}

// Thread 2
...
switch ( queue.take() ) {
  case Event:
       ...
       break;

  default:
       ...
       break;
}
OldCurmudgeon
  • 64,482
  • 16
  • 119
  • 213
  • This works, but what I do not like about it is that Thread 1 must be aware how many threads are waiting for it: If you have n Threads that are take()ing, Thread 1 needs to put n events – Kosta Dec 19 '11 at 14:11
  • @Kosta - could you expand on that requirement in your question please? I am sure we can provide that functionality too. – OldCurmudgeon Dec 19 '11 at 14:14
  • I think you might like to check out the CyclicBarrier too - this will handle all the jobs you named and does not require any lowlevel code too :) - http://docs.oracle.com/javase/6/docs/api/java/util/concurrent/CyclicBarrier.html – light_303 Dec 19 '11 at 15:08
0

Seems there is only ugly solutions. I solve it using AtomicBoolean as flag and some sleep to prevent high cpu usage and timeout for unexpected lost event...

Here my code: somewhere in thread class:

private static final int WAIT_DELAY_MS_HACK      = 5000; //ms
private static final AtomicBoolean NeedToExecute = new AtomicBoolean(false);

In working thread, that need to send wake signal:

public static final void SendSignalToExecute(){
    synchronized(NeedToExecute){
        NeedToExecute.set(true);
        NeedToExecute.notify();
    }
}

In the thread that must wait signal:

//To prevent infinite delay when notify was already lost I use WAIT_DELAY_MS_HACK in wait(). 
//To prevent false interruption on unknown reason of JM I use while and check of AtomicBoolean by NeedToExecute.get() in it.
//To prevent high CPU usage in for unknown persistant interruption in wait I use additional sleep():
while (!NeedToExecute.get()){ 
    synchronized(NeedToExecute){
        try {
            NeedToExecute.wait(WAIT_DELAY_MS_HACK); //if notify() was sent before we go into wait() but after check in while() it will lost forever... note that NeedToExecute.wait() releases the synchronized lock for other thread and re-acquires it before returning
        } catch (InterruptedException ex) { //here also may be sleep or break and return
        }
    }
    sleep(100); //if wait() will not wait - must be outside synchronized block or it may cause freeze thread with SendSignalToExecute()... :(
 }
 NeedToExecute.set(false); //revert back to reenter check in next iteration, but I use it for one waited thread it cycle "do ... wait" if you use multiple thread you need to synchronise somehow this revert