Reentrance Lock Condition

Question

public class MyLockConditionTest {
    private final Lock alock = new ReentrantLock();
    private final Condition condition = alock.newCondition();
    private String message = null;

    public void waitForCallback() {
         alock.lock();
         try {
            // wait for callback from a remote server
            condition.await();
            doSomething(message);
         } finally {
              alock.unlock();
         }
    }

    // another thread will call this method as a callback to wake up the thread called condition.await()
    public void onCallbackReceived(String message) {
         alock.lock();
         try {
            this.message = message
            condition.signal();
         } finally {
              alock.unlock();
         }
    }
}

I have this code using ReentrantLock and Condition to implement a class to wait on certain callbacks from a remote server. I tested this code and seems working but I have a few questions.

Why do I need to do alock.lock() / unlock() in onCallbackReceived(). Without calling lock() / unlock(), I was getting an IllegalState exception. I am confused because the lock is held by the caller of waitForCallback() when onCallbackReceived() invoked by another thread so alock.lock() in onCallbackReceived() will always fail.
Do I need to wrap condition.await() in waitForCallback() with while loop?

while(message==null) condition.await();

1) waiting for a condition requires a mutex to protect the condition from being changed in unsafe ways (visibility). 2) spurious wakeups amongst others, also https://computing.llnl.gov/tutorials/pthreads/#ConVarSignal for the exact same in low level — zapl, Nov 17 '15 at 19:17

score 1 · Answer 1 · answered Nov 17 '15 at 19:14

1

Why do I need to do alock.lock() / unlock() in onCallbackReceived(). Without calling lock() / unlock(), I was getting an IllegalState exception.

You are trying to signal a condition in which you do not own the lock. The only way you would hold the lock is if onCallbackReceived is called from doSomething which there is no indication that is occurring.

Do I need to wrap condition.await() in waitForCallback() with while loop?

Yes, imagine you have 5 threads stopped on the condition and 5 threads blocked on lock. Only one thread can wake up. What if when a waiting thread finally wakes up another thread had null'd that field out? You will need to check again to ensure the predicate is still true.

answered Nov 17 '15 at 19:14

John Vint

39,695
7
78
108

Thanks for your comment. onCallbackReceived is not called from doSomething doSeomthing. There is another thread has a reference to this class object and the thread calls onCallbackReceived and that was what I don't understand. the lock is hold by the caller of waitForCallback but onCallbackReceived still can acquire lock and be able to reach signal() – codereviewanskquestions Nov 17 '15 at 19:49
So, you're saying both threads are holding the `lock` simultaneously? – John Vint Nov 17 '15 at 19:51
authCallbackLock.isLocked() and authCallbackLock.isHeldByCurrentThread() give me "false " in onCallbackReceived() right before alock.lock() although authCallbackLock.isLocked() is true after alock.lock() in waitForCallback() – codereviewanskquestions Nov 17 '15 at 20:01
Things will run really really really fast. You shouldn't base success on timing of the `isLocked`. You should have both threads `Thread.sleep(1000000L)` while holding the lock and right before the sleep write a debug. – John Vint Nov 17 '15 at 22:24
If both aren't logged before going into the sleep than only one thread is succeeding. – John Vint Nov 17 '15 at 22:24

Reentrance Lock Condition

1 Answers1