1

Why in the following Java code:

public class WaitForOther {

private volatile boolean in = false;// need volatile for transitive closure
                                    // to occur



public void stoppingViaMonitorWait() throws Exception {

    final Object monitor = new Object();

    new Thread(new Runnable() {

        @Override
        public void run() {
            synchronized (monitor) {
                in = true;
                try {
                    monitor.wait(); // this releases monitor
                    //Thread.sleep(8000); //No-op
                    System.out.println("Resumed in "
                            + Thread.currentThread().getName());

                } catch (InterruptedException ignore) {/**/
                }
            }

        }
    }).start();

    System.out.println("Ready!");
    while (!in); // spin lock / busy waiting
    System.out.println("Set!");
    synchronized (monitor) {
        System.out.println("Go!");
        monitor.notifyAll();
    }

}

un-commenting of the Thread.sleep(8000); //No-op results in a shortened output:

Ready! Set! Go!

which otherwise correctly resumes in the interrupted Thread-0:

Ready!
Set!
Go!
Resumed in Thread-0

Here's the JUnit test which invokes the above behavior, as asked for in the comments:

public class WaitForOtherTest {

    WaitForOther cut  = new WaitForOther();


    @Test
    public void testStoppingViaMonitorWait() throws Exception {
        cut.stoppingViaMonitorWait();
    }



}

Thanks!

Simeon Leyzerzon
  • 18,658
  • 9
  • 54
  • 82
  • Look at handlers and postDelayed(), or an AsyncTask – zgc7009 Sep 05 '16 at 01:21
  • Possible duplicate of [Thread Signalling Sequence](http://stackoverflow.com/questions/39053847/thread-signalling-sequence) – Erwin Bolwidt Sep 05 '16 at 01:33
  • 1
    If your `notify` call completes before the `wait` started, then the `wait` will not see the previous notify and it will wait forever for a second notify that won't come. You should always use a `while (!condition)` loop around a `wait` statement (see Javadoc for `Object.wait` and above suggested duplicate) – Erwin Bolwidt Sep 05 '16 at 01:36
  • Erwin, the behavior I'm questioning is that Tread.sleep(), seemingly, doesn't let execution to proceed to printing, if sleeping is disabled - then it prints "Resumed in Thread-0". Is that what you are referring to? Would you mind illustrating how you mean the fix might look like? Thanks. – Simeon Leyzerzon Sep 05 '16 at 01:49
  • I see now that your `in` is volatile so there's no risk of a race condition here. I tried your code with and without `Thread.sleep()` and in both cases it produces the second result (that ends in "Resumed in ...") so it's not reproducible on my machine. Maybe you're running a version of the code that doesn't have `while(!in); ` ? – Erwin Bolwidt Sep 05 '16 at 02:30
  • I'm running the version of code which is posted above, however I invoke it from within an external JUnit test. Maybe that affects it somehow - but it's consistently reproducible - 100% in my case. – Simeon Leyzerzon Sep 05 '16 at 02:41
  • @SimeonLeyzerzon I suggest posting the version as a Junit test that produces this bug. As running above code in a `main` method doesn't reproduce the bug, it may be related to your code being a Junit test. – Erwin Bolwidt Sep 05 '16 at 02:55
  • @ErwinBolwidt: I posted both the complete class and its related JUnit class above. Thanks. – Simeon Leyzerzon Sep 05 '16 at 03:01

3 Answers3

2

I've tried your test in JUnit and I get the opposite from what you experienced:

  1. When Thread.sleep is commented away, the test runs fine and it prints "Resumed in <>"
  2. When Thread.sleep is in the code (actually executed), the JVM terminates and the "Resumed in ..." is not printed.

The reason is that JUnit terminates the VM when the tests are complete. It does a System.exit();. You're lucky that you get the full output in case 1., because it's printed in a separate thread, and JUnit is not waiting for that thread.

If you want to make sure that the Thread is complete before the end of the test method, either you need to have your API wait for the thread, or you need to have the test wait for the thread.

If your stoppingViaMonitorWait method returns the Thread it creates, you can wait in the test. 

@Test
public void testStoppingViaMonitorWait() throws Exception {
    Thread x = cut.stoppingViaMonitorWait();
    x.join();
}

Another option is that you inject a thread pool (an instance of ExecutorService) into the class that you're testing, have it schedule its thread on the pool (that's nicer in any case), and in your test method you can call ExecutorService.awaitTermination.

Erwin Bolwidt
  • 30,799
  • 15
  • 56
  • 79
1

Your observation is not related to the Java memory model. Try running the following code:

new Thread() {
  @Override
  public void run() {
    try {
      Thread.sleep(1000);
    } catch (InterruptedException e) {
      e.printStackTrace();
    }
    System.out.println("Will not be printed when running JUnit");
  }
}.start();

If you run it in a main method, the thread - not being a deamon thread - keeps the process alive until the sleep period is over such that the last line is printed after which the process ends.

As JUnit is testing potentially broken code, it needs to assume that a thread started from a unit test might never end. The framework will therefore not wait until all started threads terminate but explicitly terminate the Java process once all test methods returned. (Given you did not set a timeout for your tests where this time out applies additionally.) This way, the JUnit test suite is not vulnerable to broken code under test.

However, if your test suite takes longer to execute than the started thread's runtime, you can still see the printed statement. For example by adding another test:

public class WaitForOtherTest {

  WaitForOther cut = new WaitForOther();

  @Test
  public void testStoppingViaMonitorWait1() throws Exception {
    cut.stoppingViaMonitorWait();
  }

  @Test
  public void testStoppingViaMonitorWait2() throws Exception {
    Thread.sleep(9000);
  }
}

you can still see the printed line.

Do however note that this print outcome is rather instable as it relies on a specific, non-deterministic test execution order and short runtimes for the non-waiting code. (Yet, it will typically work for running this example what is good enought for demonstration purposes).

Community
  • 1
  • 1
Rafael Winterhalter
  • 42,759
  • 13
  • 108
  • 192
0

This code misuses the Java's low-level thread communication constructs like wait and notify. It is not clear what you want to establish with this code.

Here are my observations with your program's behavior (these are different from yours. I ran it both within an IDE and on command line using server and client compilers):

With the sleep() //commented out//:

Ready!
thread is daemon? : false
Set!
Go!
Resumed in Thread-0

With the sleep() uncommented:

Ready!
thread is daemon? : false
Set!
Go!
<< 8 seconds pass >>
Resumed in Thread-0

Thus, it is behaving per your design: The main thread starts a non-daemon thread (say thread-0). The main thread and thread-0 then race for the lock (monitor), but thread-0 always wins out because you want it to.

If thread-0 grabs the lock, it sets the volatile write on in and immediately gives up the lock for the main thread to notify it by wait()ing on it. Now the main thread sees the volatile update (visibility guarantee), breaks out of the busy-wait, grabs the lock, prints "Go" and notifies thread-0 which, depending upon its sleep status gets notified.

The main thread won't win the race for the lock because of the busy wait.

I added a line to clarify whether your thread is daemon or not and since the JVM is not exiting because your thread is a non-daemon thread, it stays long enough for the thread to wake up from the sleep and prints its line.

It has been said that wait() should be used to only wait in a loop for a condition variable.

Kedar Mhaswade
  • 4,535
  • 2
  • 25
  • 34