java - calling notify is not waking up the other waiting thread

Question

I have two threads, T1, and T2. both has its own array list, and I want to remove one element of the array list from T1, and after its removed, I want it to want until T2 arrays list element removed as well

So this is how the flow should be:

t1 remove element
t2 remove element

t1 remove element
t2 remove element
.
.
.

This is my code:

import static java.lang.Thread.sleep;
import java.util.ArrayList;

public class ThreadsTest {

    public static void main(String[] args) {
        Thread t1 = new Thread(new T1());
        t1.start();

        Thread t2 = new Thread(new T2());
        t2.start();
    }
}

class T1 implements Runnable {

    private ArrayList<String> list;

    public T1() {
        list = new ArrayList();
        list.add("t1 1");
        list.add("t1 2");
        list.add("t1 3");
        list.add("t1 4");
        list.add("t1 5");
    }

    @Override
    public void run() {
        while (!list.isEmpty()) {
            System.out.println(list.remove(0));
            try {
                synchronized (this) {
                    wait();
                }
            } catch (InterruptedException ex) {
                ex.printStackTrace();
            }
        }
    }

}

class T2 implements Runnable {

    private ArrayList<String> list;

    public T2() {
        list = new ArrayList();
        list.add("t2 1");
        list.add("t2 2");
        list.add("t2 3");
        list.add("t2 4");
        list.add("t2 5");
    }

    @Override
    public void run() {
        while (!list.isEmpty()) {
            System.out.println(list.remove(0));
            try {

                synchronized (this) {
                    notifyAll();
                    sleep(1000);
                }

            } catch (InterruptedException ex) {
                ex.printStackTrace();
            }
        }
    }

}

Why calling notify does not wake up T1? and how to fix it?

Answer 1

There are 2 problems in your code:

First you are synchronizing/waiting on the monitor "this", which is the actual object, so in case of T1 it is T1 and for T2 it's T2. To do this properly both threads need to synchronize/wait on the same object. A common way to do this is to specifically create such an object like this:

final static Object monitor = new Object();

Please note that the final keyword is important here, you do not want to accidentally change the monitor in between.

But even if you do this, there is no guarantee that during the time T2 is calling notifyAll() that T1 is already waiting, as the order of execution with threads is undefined. Effectively that can cause T1 to deadlock during the last wait-call after remove, as T2 is already done.

Edit: How to do this with a Phaser

Declare a Phaser instance that both threads can use:

static final Phaser phaser = new Phaser(2); // 2 threads = 2 parties

The run method of T1 becomes:

while (!list.isEmpty()) {
  System.out.println(list.remove(0));
  phaser.arriveAndAwaitAdvance();
  phaser.arriveAndAwaitAdvance();
}
phaser.arriveAndDeregister();

And the run method of T2 becomes:

while (!list.isEmpty()) {
  phaser.arriveAndAwaitAdvance();
  System.out.println(list.remove(0));
  phaser.arriveAndAwaitAdvance();
}
phaser.arriveAndDeregister();

The deregister is not necessary in this case, but acts as a safety mechanism: once deregistered the system can continue without the thread that just finished.