Android notify() method is not called

Question

I'm trying to use notify() and wait(). Here is my desired class. I have a problem, when I try to call addNewItem(). If I call tryToReadItem() first and then call addNewItem() method, that log will not be printed. Note that my DemoClass is the singleton.

public class DemoClass {

private static final String TAG = "DemoClass";

private static DemoClass instance;
private Object lock = new Object();
private static Thread executor;
private static Runnable reader;
static MyQueue queue;


private DemoClass() {
    queue = MyQueue.getInstance();
    reader = new Runnable() {

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

}

public static DemoClass getInstance() {
   if (null == instance) {
        instance = new RequestExecutor();
        executor = new Thread(reader);
        executor.run();
    }
    return instance;
}

public boolean addNewItem() {
    synchronized (lock) {
        lock.notify();  // executor will be run
        Log.i(TAG, "executor run...");
    }
    return true;
}

public void tryToReadItem() {

    try {
        while (true) {
            synchronized (lock) {
                if (queue.checkTopValue() == null) {
                    Log.v(TAG, "queue is empty");
                    lock.wait();
                } else {
                    //TODO other code...
                }

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

Here is the usage of that class:

DemoClass executor = DemoClass.getInstance();
boolean bool = executor.addNewItem();

Am I missing something?

Edit: I just changed my code. Now tryToReadRequest() is executed continuously while queue is not empty. but my problem is that the line lock.notify(); does not execute.

Answer 1

There are many problems with this code

First of all

        if (queue.checkTopValue() == null) {
            Log.v(TAG, "queue is empty");
            lock.wait();
        }

Depends on official documentation

Note: Always invoke wait inside a loop that tests for the condition being waited for. Don't assume that the interrupt was for the particular condition you were waiting for, or that the condition is still true.

Your DemoClass is Singleton. But not thread safe Singleton

because multiple threads can pass null == instance condition at the same time

   if (null == instance) {
        instance = new RequestExecutor();
        executor = new Thread(reader);
        executor.run();
    }

Right way is additional check in synchronized block and using volatile instance.

so add volatile to instance

private static volatile DemoClass instance;

and rewrite getInstance() method to something like this

public static DemoClass getInstance() {
    DemoClass localInstance = instance;

    if (localInstance == null) {
        synchronized (DemoClass.class) {
            localInstance = instance;
            if (localInstance == null) {
                localInstance = new DemoClass();
                instance = localInstance;
                executor = new Thread(reader);
                executor.run();
            }
        }
    }
    return localInstance;
}

note, you can leave only check inside synchronized block, but that will make getInstance method too slow.