unlock method

[Mike Yu]

Hi,
In the unlock method of Flight By Night, some people used a condition, but others not. For example:
<code>
private HashSet lockedRec=new HashSet();
public synchronized void unlock(int recNum){
if(lockedRec.contains(new Integer(recNum)){
lockedRec.remove(new Integer(recNum);
notifyAll();
}
}
</code>
I think we should not use a condition because if the condition is false, then it will not notify other thread.
Can you give your opinion?

[Bharat Ruparel]

Hello Mike,
I suggest you modify your code as follows:

private HashSet lockedRec=new HashSet();
public synchronized void unlock(int recNum){
      if(lockedRec.contains(new Integer(recNum)){
         lockedRec.remove(new Integer(recNum);
      }
      notifyAll();
}

Regards.
Bharat

[Andrew Monkhouse]

Hi Mike & Bharat,
I like the original code.
Why are you calling notifyAll() if you have not changed any of the locks?
Regards, Andrew

[Mike Yu]

Hi Andrew,
Before a thread execute unlock method, the thread obtains a lock on this object. If the thread doesn't call notifyAll(), then other threads don't know that this lock is available.
Please comment.

[Vlad Rabkin]

Hi Mike:

 if(lockedRec.contains(new Integer(recNum) 

if doesn't contain a lock, it means it was not locked, so you don't need to invoke notifyAll(). So, Andrew suggest to move notifyAll inside you if block, instead of having it outside.
Best,
Vlad

[Philippe Maquet]

I prefer a third version :

private HashSet lockedRec = new HashSet();
public synchronized void unlock(int recNum) {      
    if (lockedRec.remove(new Integer(recNum))) {         
        notifyAll();
    }
}

It should be twice faster than versions 1 and 2, while notifyAll() is called only when the record was locked.
Best,
Phil.

[Bharat Ruparel]

Gentlemen,
I stand corrected.
Thank you.
Bharat

[Andrew Monkhouse]

Hi Mike,

Before a thread execute unlock method, the thread obtains a lock on this object. If the thread doesn't call notifyAll(), then other threads don't know that this lock is available.

It looks like you are confusing the mutual exclusion (mutex) lock on the object used by the synchronized blocks with the object holding our conceptual locks for our applications.
As soon as one thread has control of the mutex, no other threads can enter the synchronized block. However as soon as the first thread exits the synchronized block or calls wait() it releases control of the mutex, and any other thread that wants to use the mutex can do so. There is no need for a call to notifyAll() for another thread to get control of the mutex.
Our call to wait() has nothing to do with controlling access to the mutex. The thread that calls wait() is simply waiting until the lock in the conceptual lock object that it wants is removed. When we remove the conceptual lock, we will call notifyAll() in order to tell the waiting thread that it can try to get the conceptual lock again.
Note that when we call notify() or notifyAll() we are not telling all the waiting threads that the mutex is now available. When they wake up, they have to try and get the mutex themselves before they can attempt to create the logical lock.
Does that clear things up? Or have I confused you further?
By the way: I now prefer Philippe's version out of all those published.
Regards, Andrew

[Mike Yu]

Hi Andrew,
Thank you for your reply.

It looks like you are confusing the mutual exclusion (mutex) lock on the object used by the synchronized blocks with the object holding our conceptual locks for our applications.

I don't think that I confused the mutex lock (i.e., monitor on the object of the class that contains unlock method. Let us name it TestLock) and the conceptual lock for our application (i.e., adding elements into the lockedRec object). Also, please note that in this post, all synchronizations are on the methods, i.e., the object of TestLock, but not object lockedRec.

As soon as one thread has control of the mutex, no other threads can enter the synchronized block. However as soon as the first thread exits the synchronized block or calls wait() it releases control of the mutex, and any other thread that wants to use the mutex can do so. There is no need for a call to notifyAll() for another thread to get control of the mutex.

Assuming there are threads T1, T2, T3, T4. T1, T2, T3 have called wait() method. T4 has the monitor of TestLock object and is executing the unlock method, if T4 returns from unlock method without calling notifyAll(), how can T1, T2, t3 be awakened up to join the competition for the monitor on the TestLock object?

Our call to wait() has nothing to do with controlling access to the mutex. The thread that calls wait() is simply waiting until the lock in the conceptual lock object that it wants is removed. When we remove the conceptual lock, we will call notifyAll() in order to tell the waiting thread that it can try to get the conceptual lock again.
Note that when we call notify() or notifyAll() we are not telling all the waiting threads that the mutex is now available. When they wake up, they have to try and get the mutex themselves before they can attempt to create the logical lock.

I am a bit confused with this. After the thread (T1) called wait(), if another thread doesn't call notify() or notifyAll(), how can T1 wake up?
My understanding may be wrong, but please correct me.
[ September 18, 2003: Message edited by: Mike Yu ]

[Andrew Monkhouse]

Hi Mike,
I just realised that I have been having a discussion with you over the code that Bharat posted.
Your original code was correct (although I liked Phil's version better).
But Bharat had moved the call to notifyAll() outside of the block where a lock might have been removed.
And it was in my original response to both yourself and Bharat that I made the comment that if a lock has not been removed, then there is no need to call notifyAll().
So - your original code was correct, and your understanding is correct. It was only Bharat's modification that I was disagreeing with.
Is this OK? Or are you unsure why I was disagreeing with Bharat's modification?
Regards, Andrew