0

I was looking for a way to make one thread wait/sleep until another thread signalled that something was ready. The waiting thread should wake up, process the data that was made available, then go back to sleep until the other thread signalled again.

The simplest method I could find was Object.wait() and Object.notify(), which behaved like a semaphore initialised to value 0. However, without the synchronized statements around notify/wait, Java always threw IllegalMonitorStateException when the thread was not the monitor owner. So I simply put them around the code like shown below.

THREAD 1: running infinite loop

public class Main {
    private Handler handler; // only one instance (singleton pattern)

    public void listen() {
        while (true) {
            try {
                synchronized (handler) { 
                    handler.wait();
                    int value = handler.getSize();
                    // do something
                }
            } catch (InterruptedException e) {
                // ...
            }
        }
    }
}

THREAD 2: Some other class calls removeItem

public class Handler {

    // SINGLETON PATTERN - ONLY ONE INSTANCE

    private ArrayList<Integer> sharedList;

    private Handler() {
        sharedList = new ArrayList<>();
    }

    public void addItem(Integer i) {
        synchronized (sharedList) {
            // add to list
        }
    }

    public void removeItem(int i) {
        synchronized (sharedList) {
            // remove item

            // notify that something is removed
            synchronized (this) {
                this.notify(); // this == handler
            }
        }
    }

    public int getSize() {
        synchronized (sharedList) {
            return sharedList.size();
        }
    }
}

It seems to work perfectly fine but not sure if there is a hidden bug. My question is: Is this safe? Does wait release the instance lock for handler/this so notify can acquire the lock?

5
  • 1. Yes, wait explicitly releases the lock so other threads can use the class ("explicitly" means it's in the docs somewhere, I just didn't look it up). Commented Apr 6, 2020 at 15:17
  • There's kind of a hidden bug in that Main uses the lock externally, but all of Handler's methods encapsulate the lock. That's weird. I would be consistent in my encapsulation and make listen() a method in Handler. Commented Apr 6, 2020 at 15:19
  • And I disagree with Software Engineer. Synchronizing on this is quite common. There's reasons to not do it, but there's probably equal reasons to do it. For example, when you take the lock of Handler in Main, there's valid reasons why sometimes you need to be able to do that. You should however document when you synchronize on this because it's now basically part of your object's public API. Commented Apr 6, 2020 at 15:22
  • 1
    When you wait for something, you need to check for that something. This check must happen within the same synchronized block. As the documentation says, the check and wait should even happen in a loop, before acting on the condition, still while holding the lock as otherwise, there is no guaranty that the condition still holds. Since it’s hard to believe that your // do something does something useful with only having the size, it’s very likely that whatever happens there, is broken. Commented Apr 6, 2020 at 16:11
  • Re, "[wait() and notify()...] behaved like a semaphore." NO! A Semaphore remembers that it was signalled. wait() and notify() have no memory. If some thread A calls o.notify() at a moment when no other thread is waiting, then notify does nothing at all. If some other thread B subsequently calls o.wait() then it will wait until the next time (if any) that some other thread calls o.notify(). Commented Apr 6, 2020 at 17:10

1 Answer 1

1

Synchronized blocks are safe. The statement synchronized(obj) acquires the lock of the argument obj, so you can call wait and notify on it. They both require that the current thread holds the lock on the object.

You have to be careful about the double-locking you have in removeItem where you lock two objects. If you ever need this, you have to make sure that you always lock them in the same order, otherwise, you may create a deadlock.

Sign up to request clarification or add additional context in comments.

1 Comment

I didn't notice the OP was locking both this and sharedList. That's a good catch. As long as this is always held when sharedList is accessed (read or written) there's no need to lock also sharedList. The this lock will suffice.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.