Alexey Balchunas
Alexey Balchunas

Reputation: 400

The code works with notifyAll, but doesn't with notify

I'm trying to create a simple "Ping-Pong" program with two threads (the Pong thread prints his message only after the Ping thread).

The question is why the following code gets stuck all the time, but works just fine with notifyAll?

Here is my code:

public class Main {
private static class Ping extends Thread {
    private int rounds;
    Ping(int rounds) { this.rounds = rounds; }
    @Override
    public void run() {
        try {
            synchronized(this) {
                while(rounds > 0) {
                    System.out.println("Ping");
                    notify();
                    wait();
                    --rounds;
                }
                notify();
            }
            System.out.println("Ping done");
        } catch(Exception ignored) { ignored.printStackTrace(); }
    }

    public boolean isDone() { return rounds <= 0; }
}

private static class Pong extends Thread {
    private final Ping ping;
    Pong(Ping ping) { this.ping = ping; }
    @Override
    public void run() {
        try {
            synchronized(ping) {
                while(!ping.isDone()) {
                    System.out.println("Pong");
                    ping.notify();
                    ping.wait();
                }
            }
            System.out.println("Pong done");
        } catch(Exception ignored) { ignored.printStackTrace(); }
    }
}

public static void main(String[] args) throws Exception {
    Ping ping = new Ping(15);
    Pong pong = new Pong(ping);

    ping.start();
    pong.start();

    ping.join();
    pong.join();
}
}

Upvotes: 1

Views: 168

Answers (1)

Evgeniy Dorofeev
Evgeniy Dorofeev

Reputation: 136162

Remove ping.join from the code and it will work. ping.join makes the main thread wait on ping instance so you have 2 threads waiting on ping. This is why it works only with notifyAll.

Actually joins are not needed, Java will wait for both Ping and Pong to terminate anyway

If you used a separate lock for synchronization there would be no problem of this kind. Synchronizing on a Thread (Ping is a Thread) was a bad idea

Threads coordination is unreliable, it depends on thread scheduler. This

notify();
wait();

may be a problem. Imagine you send notify() and while the current thread is moving to wait() another thread wakes up does its job and sends notify() too, if current thread hasnt reached wait() yet the notification will be lost. You can easily emulate this scenario to see that it's true.

Upvotes: 2

Related Questions