0

Code snippet:

class Counter implements Runnable {
    Object s = new Object();

    @Override
    public void run() {
        try {
            synchronized (s) {
                s.wait(10000);
            }
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
        //...do Something
    }

    public void stopCounter() {
        synchronized (s) {
            s.notifyAll();
        }   
    }
}

Irrespective of whether i call stopCounter or not, the ...do Something code always executes only after the wait interval. Even after notify it still waits for 10 secs.

amit
  • 175,853
  • 27
  • 231
  • 333
Rakesh
  • 133
  • 1
  • 10
  • Do you change the variable `s` before invoking `stopCounter()`? – amit Sep 04 '12 at 12:17
  • Calls to wait should almost always be in a loop due to spurious wakeup. Basically, wait will always return when you call notify or notifyAll for the same object. But it might also return for any reason or no reason at all. So you need some sort of signal (usually a volatile boolean variable) and a loop. Each time wait returns, check the variable and if it didn't reach the desired value, wait again. – John Watts Sep 04 '12 at 12:18
  • 1
    If your goal is to usually wait 10 seconds but sometimes wakeup early, you might be better off with Thread.sleep (which has no spurious wakeup) and Thread.interrupt (when you want the sleeping thread to wake up). – John Watts Sep 04 '12 at 12:20
  • No I'm using same instance of counter. – Rakesh Sep 04 '12 at 12:28

3 Answers3

1

I cannot tell from your example what you are trying to achieve. If it is to try and replace some sort of polling then consider the BlockingQueue interface that was released in Java 5. Since that has appeared I have had no need for wait/notify. It's a lot more simple to use and java behind the scenes does the equivalent of the wait/notify for you.

RNJ
  • 15,272
  • 18
  • 86
  • 131
0

It depends of the way you use it. I have just tried it by adding a main method and running it and it seems like the wait / notify mechanism is working fine, not the way you described it. Please try it yourself:

public static void main(String[] args) {
  Counter c = new Counter();
  new Thread(c).start();
  try {
    Thread.sleep(1000);
  } catch (InterruptedException e) {
    // TODO Auto-generated catch block
    e.printStackTrace();
  }
  c.stopCounter();
}
Dan D.
  • 32,246
  • 5
  • 63
  • 79
0

My guess is that you call the run and stopCounter methods on different instances of your Counter class. They therefore use different monitors (your s = new Object()) and the call to stop won't notify the other Counter.

For example, this would behave similarly to what you describe (unless you get a spurious wakeup):

public static void main(String[] args) throws InterruptedException {
    Counter c = new Counter();
    new Thread(c).start();
    Thread.sleep(200);
    new Counter().stopCounter();
}

static class Counter implements Runnable {

    Object s = new Object();

    @Override
    public void run() {
        try {
            System.out.println("in");
            synchronized (s) {
                s.wait(10000);
            }
            System.out.println("out");
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
        //...do Something
    }

    public void stopCounter() {
        synchronized (s) {
            s.notifyAll();
        }
        System.out.println("notified");
    }
}
assylias
  • 321,522
  • 82
  • 660
  • 783
  • No I'm using the same instance, I also executed the above code. The "notified" is printed immediately but "out" is printed after 10 secs – Rakesh Sep 04 '12 at 12:27
  • I suggest you show all the relevant code so that we can reproduce the behaviour you observe. – assylias Sep 04 '12 at 12:29
  • I executed the code you pasted, I expect just after "notified" is printed "out" should be printed. But it gets printed after 10 secs. Does it get printed immediately for you? – Rakesh Sep 04 '12 at 12:32
  • No, `out` is printed after 10 seconds on my machine, which is the expected behaviour of my code. – assylias Sep 04 '12 at 12:33
  • If you replace `new Counter().stopCounter();` by `c.stopCounter();` then `out` gets printed immediately after `notified`. – assylias Sep 04 '12 at 12:34
  • I tried with your code snippet, if I remove the Thread.sleep(200); from the code it still takes 10 secs to print out. That was what i was missing, now I added Thread.sleep(100) & it works fine. But didn't get why it doesn't work without Thread.sleep. – Rakesh Sep 04 '12 at 13:04
  • let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/16248/discussion-between-rakesh-and-assylias) – Rakesh Sep 04 '12 at 13:09
  • 1
    @Rakesh Sorry no time for a chat now. But what probably happens is that you call notify before you call wait (not necessarily in your program order, but that's the way your machine orders the instructions in the various threads) - so once you call wait, because the notify event is already gone, it waits until the timeout. – assylias Sep 04 '12 at 13:22
  • Also, unless you are doing some tests, you should use the java.util.concurrent API instead of the low-level, error-prone (as you just noticed) thread API. – assylias Sep 04 '12 at 13:25