Implementing Lock/Unlock

[Samual Harvey]

Now I am starting the design for implementing lock/unlock. After doing a lot of reading on this forum, here is what I have come up: -
A brief about my Design so far:-

I am using a ConnectionInterface and a DataInterface. The ConnectionFactory on the server creates a new ConnectionInterface and the getConnection of ConnectionInterface returns instance of DataAccessRemote typecasted to DataInterface. DataAccessRemote implements DataInterface remotely.

1. I can create a LockManager class which will store the mappings of record and unique client ID(this object for DataAccessRemote) using Hashtable.
2. Inside DataAccessRemote create a protected static LockManager.
3. The lock(int record) method implementation inside DataAccessRemote will call the LockManager.lock(record, this). The lock manager lock method will check if the record requested for locking is not locked byt another client by scanning thru the Hashtable. If not it will perform the locking else "SHOULD I DISPLAY AN ERROR MESAGE TO THE USER"?
4. Same vice-versa for unlocking. Make sure that the lock is owned by the client who requests to unlock it.

I have some questions:-
1. Does this design looks OK?
2. On the client side when do I have to initiate the lock record and when to unlock the record?
3. Should LockManager object instance be part of DataAccessRemote or ConnectionFactory?
4. Do I have make methods inside LockManager, like lock(record, this) or unloack(record, this) or lock(-1, this), unlock(-1, this) syncronized .

Thanks.
[ August 16, 2002: Message edited by: Samual Harvey ]

[Michael Morris]

Hi Samual,

Does this design looks OK?

Well considering that it's almost identical to my design, I would be hard pressed to say it didn't. One thing though, use a HashMap instead of the older HashTable. Also, under point three: "SHOULD I DISPLAY AN ERROR MESAGE TO THE USER", you should throw an IOException as described for lock in the Data class and just ignore it in unlock.

2. On the client side when do I have to initiate the lock record and when to unlock the record?

The only time to lock a record on the client is just prior to booking a flight. If you are using a Facade to interact with the Data interface, then you'll probably have a method like "doBooking" which will do the sequence lock, read, modify, unlock. If when you do your read, there aren't enough seats left to finish the transaction, you will not modify but just unlock and notify the user. If in your local implementation class you leave lock and unlock as just no-ops, then you can do the same sequence, even though the read is redundant.

3. Should LockManager object instance be part of DataAccessRemote or ConnectionFactory?

I would say DataAccessRemote, since the job of the ConnectionFactory is just to provide connections to requesting clients.

2. Inside DataAccessRemote create a protected static LockManager.

You don't necessarily have to do it that way. Actually what I did was have a protected static Map of LockManagers because I was using a Builder Pattern to build composites and new composite types are created through inheritance. If you only have one LockManager then it should probably be private and not static.
Hope this helps,
Michael Morris

[John Smith]

The lock manager lock method will check if the record requested for locking is not locked byt another client by scanning thru the Hashtable. If not it will perform the locking else "SHOULD I DISPLAY AN ERROR MESAGE TO THE USER"?

... else you should wait() until that record is unlock()ed, as indicated by the absense of the record in your locks collection.
I am sure Michael did the same in his implementation, he just made an error in his response above (but let's forgive him because he gave 100 great responses in the last few weeks)
Eugene.

[Michael Morris]

... else you should wait() until that record is unlock()ed, as indicated by the absense of the record in your locks collection.

I misunderstood the question. No doubt if the record is already locked you should wait() for the record to be unlock()ed. I was addressing the requirement that lock should throw IOException (as indicated in the javadoc comment for lock) if the record position is invalid.
Sorry for the confusion,
Michael Morris

[Samual Harvey]

What does wait() here means? I am thinking it is not thread wait(). How long does the client has to wait? Can you clarify more on this.
Also,
Do I have to make my methods inside LockManager, like lock(record, this) or unloack(record, this) or lock(-1, this), unlock(-1, this) syncronized .
I think I should make these methods synchronized if I use HashMap as it is not synchronized as compared to Hashtable which is.
Thanks.

[Nate Johnson]

Please make access to your locks (hash map or the methods) synchronized!!! Just because the data type is synchroized does not mean it wont mess up... look up some posts from Peter about vectors for a better explanation. And move up to the HashMap rather than the Hashtable.
[ August 16, 2002: Message edited by: Nate Johnson ]

[Michael Morris]

Hi Samuel,

What does wait() here means? I am thinking it is not thread wait(). How long does the client has to wait? Can you clarify more on this.

wait(), notify() and notifyAll() are methods defined in Object, the granddaddy of all Java classes. They can only be called within synchronized blocks. If you don't know how to use these, you need to find a tutorial on object monitors and thread synchronization. Basically, your lock method should be synchronized and if a record is already locked you should call wait(). Your unlock() should verify that the calling object owns the lock on the record it is requesting be unlocked and if it does then unlock should remove it from the HashMap. Whether a record is removed or not, unlock() should always call notifyAll(). For every wait() there should be a notify[All]() to prevent dead lock.
Hope this helps,
Michael Morris

[Samual Harvey]

Thanks Nate and Michael,
Nate you are right I am going to use HashMap.
Yes I am aware of the wait(), notify() and notifyAll() methods on Object. I was not sure if this is what you guys were aiming at. But now it is OK. I may need a little brush up on the inner details of the subject.
So should all these wait(), notify(), notifyAll() be used inside the LockManager class or inside the client starting class? Where should I define the run method and who should invoke start()?
[ August 16, 2002: Message edited by: Samual Harvey ]

[Nate Johnson]

Originally posted by Samual Harvey:

So should all these wait(), notify(), notifyAll() be used inside the LockManager class or inside the client starting class? Where should I define the run method and who should invoke start()?

You should but the wait stuff in your lock manager... I am not sure what you are referring to for the run and start methods?

[Samual Harvey]

Originally posted by Nate Johnson:

You should but the wait stuff in your lock manager... I am not sure what you are referring to for the run and start methods?

Do you mean I should use notify(), notifyAll() inside my LockManager and wait outside. I mean does wait() has to be invoked by the client which allows the client to wait for the lock to be released. And when the LockManager is done unlock() it calls notify. Also the lock() method calls notify.
start() method starts the thread and the run() method gets executed.

[Nate Johnson]

I have a HashMap that I synchronize on in my LockManager. When I lock, I check to see if another client has the lock and if so the HashMap calls the wait(). In the unlock, notifyAll() is called by the HashMap.
I understand how threads work, I was just not sure what thread you are talking about starting up

[CyJenny Wong]

Hi Nate,
Please see below portion of code in my lockManager
Anything wrong??

  
public class LockManager {
  private HashMap lockedRecords = new HashMap();
  public void lock(int rec, Object owner)  {
   synchronized(lockedRecords) {

      while (hashmap contains the recNum which
           is locked by this owner) {
         try {
              wait(); -- or lockedRecords.wait();
 }
  
         catch (InterruptedException ie) {
      ie.printStackTrace();
 }
      }
   }
}

[Nate Johnson]

Since you are synching on lockedRecords, you would want to use
lockedRecords.wait()
just calling wait() would invoke wait() on "this" and you could run into problems...
I think I would use an if rather than while as well... but it really should not matter I guess... do others have an opinion on this matter?
Just make sure you add your record to the map when you are done wait()-ing too

[Jim Bedenbaugh]

Originally posted by Michael Morris:
Hi Samuel,
Basically, your lock method should be synchronized and if a record is already locked you should call wait(). Your unlock() should verify that the calling object owns the lock on the record it is requesting be unlocked and if it does then unlock should remove it from the HashMap. Whether a record is removed or not, unlock() should always call notifyAll(). For every wait() there should be a notify[All]() to prevent dead lock.

Call wait() and notifyAll()? All I did was try to lock the record. If I couldn't, I threw an exception, poppped up a JDialog and told the user to try again later. I figured that the record lock could have been caused by a crashed client and I didn't want then waiting around until the server died. I didn't implement a staleLock method. The specs didn't really seem to require a staleLock check - I read of folks who submitted and passed without it.
Comments?

[Samual Harvey]

  
public class LockManager {
  private HashMap lockedRecords = new HashMap();
  public void lock(int rec, Object owner)  {
   synchronized(lockedRecords) {

      while (hashmap contains the recNum which
           is locked by this owner) {
         try {
              wait(); -- or lockedRecords.wait();
 }
  
         catch (InterruptedException ie) {
      ie.printStackTrace();
 }
      }
   }
}

[/qb]<hr></blockquote>
I have a question on this:-
If we create a new lockmanager object instance for each unique client connection. Then each lockManager will have its own HasMap object i.e. lockedRecords. In this case how will it know if there is a record locked by another client. Because everytime there there will be a new HashMap object.
I was going thru the RHE book 2E and it says the other technique where you synchronize the whole method is better then synchronizing a block of code. I am still working on a thought to put together the wait and notify stuff...
Also I was thinking do we need this:-
Map m = Collections.synchronizedMap(new HashMap(...));
[ August 16, 2002: Message edited by: Samual Harvey ]

[Nate Johnson]

I did miss that lockRecords was not static. It should be so that all clients know about it.
About a synched-map... I think it is a waste of processor time to synch the map... you still have to use synchronized blocks or methods for doing the locking/unlocking, so why not use a faster datatype?

[Samual Harvey]

Also we have to synchronize the unlock method.
As it releases the lock i.e remove the information from the hashMap and does notifyAll().

[Nate Johnson]

I used synchronized blocks, but yes, that is what I did for my unlock() method.

[Samual Harvey]

I am trying this it goes into waiting buit even though I invoke the unlock it does not comes out of waiting..whats wrong I am doing here

private static HashMap hashMap = new HashMap();
    public void lock(int record, Object clientID){
        synchronized(hashMap){
            //if hashmap has the record locked by another client ID
            try{
                System.out.println("Waiting::::::::");
                hashMap.wait();
            }
            catch(InterruptedException e){
            }
            System.out.println("Out of Waiting::::::::Going to add record to hashmap");
            //add the record infor in the HashMap.
        }
    }
    public void unlock(int record, Object clientID){
        synchronized(hashMap){
            // if hashmap contains the lock then delete that entry
            System.out.println("Going to unlock all the records::::::::");
            hashMap.notifyAll();
        }
    }

On the client side I call lock and then unlock and on the server it keeps displaying "waiting:::" it does not come out of waiting....
[ August 17, 2002: Message edited by: Samual Harvey ]

[Michael Morris]

Originally posted by Jim Bedenbaugh:
Call wait() and notifyAll()? All I did was try to lock the record. If I couldn't, I threw an exception, poppped up a JDialog and told the user to try again later. I figured that the record lock could have been caused by a crashed client and I didn't want then waiting around until the server died. I didn't implement a staleLock method. The specs didn't really seem to require a staleLock check - I read of folks who submitted and passed without it.

Well the instructions say that if a record is already locked then the lock method should block, not throw an exception. Block implies wait(). To deal with stale clients, your remote implementation class should implement the Unreferenced interface, which is called sometime after the remote gc determines that there are no reachable references to the remote object. So in the unreferenced method, you simply call unlock on all the records that the remote implementation class holds.
Hope this helps,
Michael Morris

[Jim Bedenbaugh]

Originally posted by Michael Morris:

Well the instructions say that if a record is already locked then the lock method should block, not throw an exception. Block implies wait().
To deal with stale clients, your remote implementation class should implement the Unreferenced interface, which is called sometime after the remote gc determines that there are no reachable references to the remote object. So in the unreferenced method, you simply call unlock on all the records that the remote implementation class holds.

Yep. It sure does. The way I understood the requirement was that the lock method should be synchronized, preventing collisions on my HashMap which holds the locked record keys. Oh well. A simple fix.
Thanks.

[Samual Harvey]

Well the instructions say that if a record is already locked then the lock method should block, not throw an exception. Block implies wait().

This is what I tried in the above code. It goes into waiting and then even if I call notifyAll() it does not come out of waiting. I am not sure whats wrong.

To deal with stale clients, your remote implementation class should implement the Unreferenced interface, which is called sometime after the remote gc determines that there are no reachable references to the remote object. So in the unreferenced method, you simply call unlock on all the records that the remote implementation class holds.

I am not sure how to do this at this time. What do you mean by Unreferenced Interface here? I will spend sometime on this after I fix basic lock/unlock problem I am having above.
Thanks.
[ August 17, 2002: Message edited by: Samual Harvey ]
[ August 17, 2002: Message edited by: Samual Harvey ]

[Nate Johnson]

Originally posted by Samual Harvey:

What do you mean by Unreferenced Interface here? I will spend sometime on this after I fix basic lock/unlock problem I am having above.

There is an interface called Unreference that has one method, unreferenced(). My RemoteData object implements this interface and uses it to call the LockManager (I have a cleanDeadClientLocks(Object client) method in there too) and remove all locks by this RemoteData object when the method is called (which probably means the client crashed).
[ August 17, 2002: Message edited by: Nate Johnson ]

[Samual Harvey]

Does the lock manager has to extend thread class or implement runnable. I am not doing that right now and I see that my code above just goes in waiting. I think once it goes into waiting the next call to unlock does not gets executed.
What am I missing in this code: -

private static HashMap hashMap = new HashMap();
    public void lock(int record, Object clientID){
        synchronized(hashMap){
            //if hashmap has the record locked by another client ID
            try{
                System.out.println("Waiting::::::::");
                hashMap.wait();
            }
            catch(InterruptedException e){
            }
            System.out.println("Out of Waiting::::::::Going to add record to hashmap");
            //add the record infor in the HashMap.
        }
    }
    public void unlock(int record, Object clientID){
        synchronized(hashMap){
            // if hashmap contains the lock then delete that entry
            System.out.println("Going to unlock all the records::::::::");
            hashMap.notifyAll();
        }
    }

[ August 17, 2002: Message edited by: Samual Harvey ]

[Michael Morris]

Hi Samual,

Does the lock manager has to extend thread class or implement runnable

No. The threads which will run in LockManager will be created and started by RMI.

Michael Morris

[Michael Morris]

Hi Samual,
The reason that your application is getting hung in a wait state is probably because you only have one thread running. How can notifyAll() ever be called? To test it, you will need at least two threads and you will have to make sure that the two threads never call lock one after the other (at least until you have a deterministic control in place on the lock method.) So if you set it for thread 1 to call lock, then sometimes later thread 2 calls unlock, then thread 1 should "get the ball" again and proceed.
Hope this helps,
Michael Morris

[Samual Harvey]

Originally posted by Michael Morris:

No. The threads which will run in LockManager will be created and started by RMI.

Can you explain this. How can there be threads inside the LockManager when the LockManager itself does not extend thread or implement runnable. Is this another class inside LockManager which extends thread or implement runnable. Or this is a thread started by DataAccessRemote which runs the LockManager?

Thanks.
[ August 17, 2002: Message edited by: Samual Harvey ]

[Michael Morris]

Hi Samual,
Any number of threads can run in any object. That object need not be a Runnable object. For example:

    public class Runner implements Runnable {
        private LockMangaer lockManager;
        private int id;
        public Runner(LockManager lockManager, int id) {
            this.lockManager = lockManager;
            this.id= id;
        }
        public void run() {
            for (int i = 0; i < 10; i++) {
                lockManager.lock(i + 1, this);
                System.out.println("Thread " + id + "Locking record: " + (i + 1));
                lockManager.unlock(i + 1, this);
                System.out.println("Thread " + id + "Unlocking record: " + (i + 1));
            }
            System.out.println("Thread " + id + "Done locking");
        }
        public static void main(String[] args) {
            LockManager lockManager = new LockManager();
            Runner[] runners = new Runner[10];
            for (int i = 0; i < 10; i++) {
                runners[i] = new Runner(lockManager, i + 1);
                (new Thread(runners[i])).start();
            }
        }
    }   

That should start ten threads which will each lock and unlock records 1 thru 10. As it runs, you may notice that the threads do not run in order or they may. The point is all ten threads will run in the LockManager even though LockManager itself is not a Runnable object. The thing to remember is as I stated earlier, any number of threads may run in any object.
I just threw this code down, so if you want to test it out and there is a problem let me know and I'll actually run it here.
Hope this clears it up,
Michael Morris
[ August 17, 2002: Message edited by: Michael Morris ]

[Samual Harvey]

I guess my question was that somebody has to either extend Thread or implement Runnable. In this case you have a class outside the LockManager who implement Runnable. I am going to try this right now.

Thanks,

[ August 18, 2002: Message edited by: Samual Harvey ]

[Samual Harvey]

OK, I tried this and it appears to be working partially. I am sure I am doing something wrong here.
Here are some points:-
1. The server can now wait() after acquiring the lock and can continue once unlock does the notifyAll(). This part seems to be OK
2. But the client after issuing the lock does not wait and contines even though the server is still waiting on the lock. This part I am not sure if correct. I think this should wait.
This is what I did:-
1. I created a RunThread class and in the run() method called lockManager.lock() or lockManager.unlock() based on how this is been called from the DataAccessRemote class. The constructor of RunThread class starts the thread using itself as an object by new Thread(this).start().
2. Inside DataAccessRemote class I created objects for LockManager. And based on if the client calls lock then I created the RunThread object using the LockManager instance, DataAccessRemote instance, record number to be locked and lock type.
But the DataAccessRemote does not wait after creating the RunThread object.
How can I make the DataAccessRemote Object wait here as long as the LockManager is waiting on the lock.
Thanks.
[ August 18, 2002: Message edited by: Samual Harvey ]

[cindy sung]

Hi Samual:
I think you are going the wrong way. You are making it more and more complex but further and further away from the standard way of implementing a lock manager.
Actually you just need a monitor to control the mutual exclusive access to each record in the database. And in this project, LockManager is the monitor.
So, first, you cannot use

synchronised(hashMap)

since that is equivalent to lock the entire database, which is not desired. Second, LockManager must be a singleton ( I don't know if you used the singleton pattern, but if you use it, then follow the rest points I stated below). Third, make lock and unlock method synchronized instead of synchronizing on objects. Then, still use wait and notifyall pairs to implement the semaphore.

cindy

[Samual Harvey]

I think the idea behind the logic is to create mutliple threads that launch LockManager. The LockManager itself then controls access to the lock/unlock emthods using the synchroinzed code. Here the monitor is the hashMap.
Even if I make LockManager singleton and can guarantee to have only one instance of LockManager it will not address the problem where my client is not waiting while the LockManager is waiting.
I am not sure if synchronized(hashMap) can lock the whole database. Anyway, I'll keep working on it and will post more details.
Michael, Nate do you guys have comments on this.
Thanks.

[Michael Morris]

Hi Samual,

I think the idea behind the logic is to create mutliple threads that launch LockManager.

Just for testing. Remember, in the completed application RMI will create the client threads that run in your lock manager. And Cindy is right, there should only be one lock manager (per Data object) or only one Map that holds the locked records. But I would be wary of implementing it as a true Singleton. The Singleton Pattern is probably the most abused design pattern there is and as long as you are sure that you only create one, that should be sufficient. If you prefer, you can create a new lock manager for each client if you make your Map static. In that case, it would then be necessary to synch on the Map instead of the method. Otherwise, I agree with Cindy that you should synchronize the whole method. That is always a safer approach. When synchronizing on referenced objects in a method, there is a much more likely probability of creating a dead lock, especially if other objects hold a reference to the object you are synching.

I am not sure if synchronized(hashMap) can lock the whole database. Anyway, I'll keep working on it and will post more details.

You can do it either way. There are two schools of thought on locking the whole database: loop through all the records and lock them (this was Mark's approach); set a "dblock" flag, that when set, will not allow any further records to be locked and wait for all pending locks to be unlocked at which time the dblock is established (this was my and I believe Nate's and Eugene's approach).
Hope this helps,
Micahel Morris
[ August 18, 2002: Message edited by: Michael Morris ]

[Samual Harvey]

It seems to be working now. Thanks for a detailed analytical reply. It is much clear now.
Let me ask you :-
Can I combine both singleton and syn code. Is there a problem if I make LockManager a singleton and make sure only one instance is created and then use sync code inside lock and unlock rather then making the whole method syn.
Yes, I later realized that I should put the lockManager object instance inside the DataAccessRemote constructor so when the RMI creats a remote object it also creates an instance of lockmanager.
Thanks.

[Michael Morris]

Hi Samual,

Can I combine both singleton and syn code. Is there a problem if I make LockManager a singleton and make sure only one instance is created and then use sync code inside lock and unlock rather then making the whole method syn.

Sure. There is nothing special about a singleton other than like Tigger "It's the only one."
Hope this helps,
Michael Morris

[Samual Harvey]

On implementing Unreference Interface, I found this topic. Do you think its a good idea to do.
Past Discussion
Also, I have this question on how to determine and handle the vice-versa. I mean how can the clients know if the server has shutdown or crashed?
[ August 18, 2002: Message edited by: Samual Harvey ]

[Nate Johnson]

Originally posted by Samual Harvey:
On implementing Unreference Interface, I found this topic. Do you think its a good idea to do.
Also, I have this question on how to determine and handle the vice-versa. I mean how can the clients know if the server has shutdown or crashed?

I think it is a good idea... it is very simple to do and it is a nice improvement. But it is not a requirement.
About the server crashing... your client should get a RemoteException the next time it tries to go out and get something from the server after it crashed. It this case, the client is pretty much useless and there is nothing you can do but infor the user and send a note to the server admin to restart it

[Michael Morris]

Hi Samuel,

On implementing Unreference Interface, I found this topic. Do you think its a good idea to do.

This is a good discussion on using Unreferenced. Pay close attention to Peter's posts on the subject, he's the real guru here. My tests indicated that there was almost a fifteen minute delay from the time a client crashed until the unreferenced() method was called. At first, I thought that was too long and considered (actually even implemented) a timer in the lock manager to reclaim locks after five minutes. That's really a bad idea, becuase it assumes something that may not be true. With Unrefernced, you know for sure that the client is dead. The easy fix is to have booking occur in a background thread, set a busy cursor on that particular row in your table and booking mechanism and keep it disabled until the lock becomes available. That way if the user selects another row(s) he can continue to book flights while the pending booking waits for the lock.

Also, I have this question on how to determine and handle the vice-versa. I mean how can the clients know if the server has shutdown or crashed?

Here is how I handled it:
Server Events

As I indicated in the post, it involves some work and may not be worth the time. I enjoyed doing it and learned some neat tricks in the process. If you decide to try something like what I did, let me know and I'll walk you through the steps.
Hope this helps,
Michael Morris

[Sarita Gupta]

Hi Michael,
Well not sure abt Samuel, but i'd like to know more...
I did refer to ur Server Events. Basically, u'r sending server status to the clients.... but what's interesting is that you have a DELAYED_SHUTDOWN and a SHUTDOWN with a 60 sec time difference... any reason for that two seperate events?
Sarita
[ August 18, 2002: Message edited by: Sarita Gupta ]

[Michael Morris]

Hi Sarita,
Yep. When the DELAYED_SHUTDOWN event is sent, there was a message (String) that said: "Database server going down in 60 seconds." The idea is that, that should give clients time to finalize any pending bookings and alert them that they should not attempt any further bookings. This is similar to how the shutdown command works on UNIX systems.
The basic architechture that I used, requrires that the ServerEventService and the subscribed client object each run in background threads. Events were sent/received using the Observer Pattern, i.e. extending Observable/implementing Observer. The background thread on the client stays in a wait state most of the time. Whenever the update() method is called by the Observable (ServerEventService) then the thread is awakened with notifAll() and the event is processed by sending it to the main client application (also by using the Observer Pattern).
On the server, the ServerEventService requires addServerListener() and removeServerListener() methods and of course a fireServerEvent() method. You'll also have to set up a timer (be sure to use java.util.Timer instead of javax.swing.Timer) for the heartbeats. I used a private inner class for the TimerTask. You will also need to set up something similar on the client to verify that there is a regular heartbeat from the server. Of course you'll need a ServerEvent class which extends EventObject and which must implement Serializable for RMI transport. That's the broad overview of it. The beauty is that this event system should be recognized by a bean container, although I haven't tried it in the beanbox yet.
Hope this helps
Michael Morris