Strange java behavior of wait/notify

5
votes

I've found strange behavior of java concurrency. See on the next code below:


    public class Test {
        static CountDownLatch latch = new CountDownLatch(1);
        public static void main(String[] args) throws UnsupportedEncodingException, InterruptedException {
            final Thread t = new MyThread();
            t.start();
            synchronized (t) {
                latch.countDown();
                System.out.println("got to sleep");
                t.wait();
                System.out.println("wake up");
            }
        }

        static class MyThread extends Thread {
            @Override
            public void run() {
                try {
                    latch.await();
                } catch (InterruptedException e) {
                    e.printStackTrace();
                }
                synchronized (this) {
                    System.out.println("inside run");
    //                notifyAll();
                }
            }
        }
    }

In my point of view this code should be hang up and wait forever, but code is finished without any problem with next out in console:

got to sleep
inside run
wake up

I've tried to find some information about notifying locks if thread is died, but was lack in it. Also I've not find any information in java specification.

But if I've tried to lock on some other object (not on the thread object) it was work fine as I expected.

2
javamultithreadingconcurrency
You lock on the sync on the thread instance, run the thread, then wait on the thread in Main. At this point, the thread should have the reigns. It syncs on the this, which is the same thing Main waited on, then calls notifyAll(), which releases the wait in Main. The app should run fine. – CodeChimp
Additionally, do not call wait/notify/notifyAll on Thread - it uses it internally. Oh, and prefer implementing Runnable instead of extending Thread :) – Jon Skeet
why is notifyAll() commented out? If it is active then I would also expect the code not to hang up. – Meno Hochschild

2 Answers

8
votes

It's because you're waiting on a Thread instance. Thread uses wait/notify/notifyAll internally, so you shouldn't do that yourself as well - you'll confuse Thread, and Thread will confuse you. In particular, when a thread exits, it calls this.notifyAll().

From the documentation of Thread.join:

This implementation uses a loop of this.wait calls conditioned on this.isAlive. As a thread terminates the this.notifyAll method is invoked. It is recommended that applications not use wait, notify, or notifyAll on Thread instances.

In general, try to lock and wait for objects which nothing else can interact with. That way you can reason about the concurrency-related operations which exist on the object, because they're very limited. As soon as arbitrary code can synchronize on the object (and call wait/notify etc) it's hard to prove that your code is correct. That's why you'll often see something like:

public class Foo {
    private final Object lock = new Object();

    ... code which uses synchronized(lock) ...
}
0
votes

As Jon Skeet writes it is bad idea to synchronize on Thread object, use another one:

public class Test {
    static CountDownLatch latch = new CountDownLatch(1);
    static final Object sync = new Object();

    public static void main(String[] args) throws UnsupportedEncodingException, InterruptedException {
        final Thread t = new MyThread();
        t.start();
        synchronized (sync) {
            latch.countDown();
            System.out.println("got to sleep");
            sync.wait();
            System.out.println("wake up");
        }
    }
}

static class MyThread extends Thread {
    @Override
    public void run() {
        try {
            latch.await();
        } catch (InterruptedException e) {
            e.printStackTrace();
        }
        synchronized (sync) {
            System.out.println("inside run");
            //sync.notifyAll();
        }
    }
}

Now this code will never ending until you uncomment sync.notifyAll();