[URLyBird]Is my lock appraoch right?

[Zhixiong Pan]

Hi all,

The book() method in HotelBusiness will first call lock() in Data. In DB Layer I have a class LockManager. So lock() in Data will first call lock() in LockManager. Following is abstrac of code for LockManager. I can not convince myself, what about your idea?

  
public class LockManager{
  HashMap lockMap;
  long lock(recNO){
    while(true){
      synchronized(lockMap){
        check if the recNo already in lockMap;
      }
      if(already locked){
        synchronized(this){
          Thread.currentThread().wait();
        }
      }
      else{
        generate lockCookie;
        return lockCookie;
      }
    }
  }
  

[Andrew: Deleted methods - refer to SCJD FAQ for why.]
[ April 11, 2006: Message edited by: Andrew Monkhouse ]

[Luc Feys]

Hello Zhixiong,

I might be wrong but I think there is a securiy hole in your lock() method.

Suppose two threads A and B.

Thread A executes the code protected by "synchronized(lockMap)" and finds that the record is not locked.
Then thread B jumps in and executes the same code protected by "synchronized(lockMap)" before thread A has the possibility to really lock the record. So thread B also thinks that the record is not locked.

So both threads will proceed in locking the record (but only the last one's cookie will be valid).

You should be able to test this easily. If you would add a "Thread.sleep" between the check of the map and the actual locking, I think you would get a SecurityException thrown by at least one thread.

You should find plenty of possible ways to solve this in the forum. But it is also very well explained in Andrew's book.

I hope this was of any help.

Luc

[Mihai Radulescu]

Hi Zhixiong,

Here are some points:
1.Why you wait on thread - this can create problems if you choose to use RMI(just search RMI- are a lot of topics with this theme)
2.Consider this scenario : a thread can snack after the sync block

synchronized(lockMap){
    check if the recNo already in lockMap;
}

and lock/release some records - this can alter the lockMap -> the condition "if(already locked)" does not guarantee that the record are really locked or not.
3.You can generate a lock cookie also if the record was not locked(see point 2).
4.You will get a IllegalMonitorException because you wait using the current thread and you notify using the actual instance(I presume is not the current thread)
5.You can use the contains() methods to check if a Map contains a key
6.Your isLocked never return false !! -this is not so nice.



Regards ,Mihai

[Zhixiong Pan]

Hi Mihai,

Thanks for your reply.According your points, I want to consult with you:
1. Cause RMI will produce threads itself instead of your working, so why not use wait. Are there any potential emergency? If there are, what thread blocking approach should be used instead?
2 and 3. I modify the lock as following:

    long lock(recNO){
    while(true){
      synchronized(lockMap){
        generate lockCookie;
        check if the recNo already in lockMap;
        if(not clocked)
          lockMap.add(new Integer(recNo),lockCookie);          
      }
      if(already locked){
        synchronized(this){
          try{
            this.wait();
          }
          catch(InterruptedException e){}
          }
        }
      }
      return lockCookie;
  }  

4. not quite clear.
5. You are right!
6. I think the call of isLocked() can only get ture return or get a SecurityException.

[Mihai Radulescu]

Hi Zxiong,

Right now I am pretty busy and I try to be fast:

1.You don't know how many threads are used by the RMI - is not a bad idea to have a lock for each thread but if you choose this solution you must be shore that you can manage/manipulate this locks easily. Back to your example you use the like look the current thread - the actual current thread so when yourelease your look you must be shore that you get the same (Thread)instance. Once more RMI does not guarantee that all your methods are running under the same thread. So if you lock with a lock object be shore that you release with the same lock (object).

2&3.A Thread can sneck between your while() block and if block - lets say that a thread waits in the spin condition and it pass this - we are exact after the while block - an other thread slide in and lock the record A(lets say) , the fist thread knows that the record A is not lock(it pass the spin condition) and ....bam. More simple can happen that your lock returns a lookCookie even if the record is not really lock.
4. The wait & notify(All) must be called on the same instance your previous code snippet you use(currentTnerad.wait() and thisnotifyAll())
5.A boolean method can be return true or false - thats the boolean meaning In improper to use an exception instead of false - this make your code a little bit hard to understand.

Regards ,Mihai