[Solved] What is wrong with it?

by


The thread you create in main invokes MyThread#k() which goes into a wait. At that point, that thread will do nothing else until it is awakened or interrupted. But the only place in your code where it could possibly be awakened is the notify in MyThread#m(). Since nothing in your program calls that method, the thread can never be awoken.

What you probably want is to add a call to s.m() right after s.start() in your main program. That way your main thread will execute the notify that’s needed to wake up your thread.

Unfortunately, that’s very unlikely to work. The problem is that s.start() causes your created thread to become ready to run, but it doesn’t necessarily run immediately. It could well happen that your call to s.m() will complete before the created thread does anything. And then you’ll still have exactly the same result as before, except that you’ll see the integers 0..6 printed out before before wait. The notify will do nothing, because the child thread has not yet performed its wait. (And by the way, since both MyThread#k() and MyThread#m() are both synchronized, increasing your loop limit in MyThread#m() won’t change a thing… the child thread won’t be able to enter MyThread#k() while MyThread#m() is running. You could improve that by putting the notify in a sycnchronized block rather than making all of MyThread#m() synchronized.)

You can try to get around this by adding Thread.sleep(1000) before s.m() in your main program. That will almost certainly work because your main thread will yield execution, giving your JVM the opportunity to schedule the child thread for some useful work. By the time the main thread wakes out of its sleep and performs its s.m() call, the child will probably have executed its wait and you will then see your do something after wait message.

But that’s still pretty crummy, because it still depends on scheduling events that you don’t really have any control over. There’s still no guarantee that the wait will happen before the notify.

This is why when using wait/notify you should generally arrange for there to be some sort of reliable test as to whether whatever you’re waiting to be done has actually occurred. This should be a condition that, once it turns turns true, will remain true at least until the test has been subsequently performed. Then your typical wait loop looks something like this:

while (!isDone()) {
    synchronized(monitorObject) {
        try {
            monitorObject.wait();
        } catch (InterruptedException e) {
        }
    }
}

Putting the whole thing in a loop takes care of premature waking, e.g. due to InterruptedException.

If the required work has already occurred by the time this code is executed, no wait occurs, and the notify executed by the code that did the work was a no-op. Otherwise, this code waits, and the code completing the work will eventually do a notify which will wake this code up as required. Of course, it’s critical that, at the time the notify is performed, the wait condition (isDone() above) be true and remain true at least until tested.

Here’s a corrected version of your code that incorporates a proper wait loop. If you comment out the Thread.sleep() call, you will likely not see the waiting message, because the work will complete before the wait loop even starts. With the sleep included, you’ll probably see the waiting message. But either way, the program will work properly.

public static void main(String[] argv) throws Exception {
    MyThread s = new MyThread();
    s.start();
    Thread.sleep(1000);
    s.m();
}

class MyThread extends Thread {

    @Override
    public void run() {
        k();
    }

    private boolean done = false;

    public void k() {
        System.out.println("before wait");
        while (!done) {
            System.out.println("waiting");
            synchronized (this) {
                try {
                    wait();
                } catch (InterruptedException e) {
                }
            }
        }

        System.out.println("do something after wait");
    }

    public void m() {
        for (int i = 0; i < 6; i++) {
            System.out.println(i);
        }
        synchronized (this) {
            done = true;
            notify();
        }
    }
}

solved What is wrong with it?