• Post Reply
  • Bookmark Topic Watch Topic
  • New Topic

review lock/unlock

 
Prakash Krishnamurthy
Ranch Hand
Posts: 154
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi all,
I know there have been infinite lock/unlock review requests, this is one more. It would be very helpful if you guys could tell me if there is anything wrong. I have seen people post their code for suggestions. Bartenders(Mark and Max),If i have posted too much, let me know, I shall remove some of it.
Here is my design. Kindly let me know you comments.
The following code is in the locK manager class:
public void lock(int recno, Object thread){
if(object not hashed){
try{
lock();
}
catch(Exception ep){
ep.printStackTrace();
}
hash it
}
}
public void unlock(int recno){
if(no hash && no database lock){
unlock
remove hash;
}
else{
System.out.println("There is no lock to unlock or else the database has been locked");
}
}//end of unlock
[ April 11, 2003: Message edited by: Prakash Krishnamurthy ]
[ April 14, 2003: Message edited by: Prakash Krishnamurthy ]
 
Andrew Monkhouse
author and jackaroo
Marshal Commander
Pie
Posts: 12007
215
C++ Firefox Browser IntelliJ IDE Java Mac Oracle
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Prakash,
Please put code into UBB Code blocks.
It is STRONGLY recommended that you follow the official coding standards (coding standards gets 23 marks in the old assignment). If you do not have a book that tells you the coding standards expected, then go to Sun Java coding conventions and download a copy. Variable names and brace locations need to be modified.
I cannot see how you are handling locking the entire database, unless you are locking the entire database when an individual record is being locked - this does seem possible at a quick review of the code: writeLock appears to set the LockedThread variable (again: naming conventions), and no other thread can lock a record while that variable is set, and that variable is not cleared until the unlock() method is called.
The mechanism to ensure that locks are processed in the order in which they are received is a nice idea. It seems to fall outside of requirements, but it is nice.
Nothing in this class seems to be static, and the class itself is not a singleton. How are you handling multiple threads? From the calling class?
Your unlock(int, Object) method calls done() - should it be calling unlock()? If not, what calls unlock?
I think having two methods with the same name when one is not overriding the other is very hard to read. I think you should change unlock() to done() or some other name.
Also, the writelock() and unlock() methods appear to be internal methods for the lock(int, Object) and unlock(int, Object) methods - shoulldn't they be private then?
Uuurgh - looking at my post, I appear to be overly critical. Sorry about that. I can see where you are going with this, so all is not lost.
Good luck. Andrew
[ April 10, 2003: Message edited by: Andrew Monkhouse ]
 
Prakash Krishnamurthy
Ranch Hand
Posts: 154
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
It is STRONGLY recommended that you follow the official coding standards (coding standards gets 23 marks in the old assignment).

I shall make a note of that. I did indent the code, but for some reason, when I stuck the code from notepad, the indentation just went away and I did not notice it. But point noted and thanks a lot
Nothing in this class seems to be static, and the class itself is not a singleton. How are you handling multiple threads? From the calling class?

Can you please explain a little bit. I am not sure what you are trying to say. when you ask "how are you handling multiple threads from the calling class?". Do you mean the class calling Readwritelock class or do you mean LockManager class?
Your unlock(int, Object) method calls done() - should it be calling unlock()? If not, what calls unlock?

I am sorry, my mistake here. I should actually be calling done. The code does call done. I was trying to delete some System.out.println statements before posting the code here and I must have "mistyped" that somehow.
I cannot see how you are handling locking the entire database,

I did not post that code, because I thought as it is I was posting too much of code :-)
 
Andrew Monkhouse
author and jackaroo
Marshal Commander
Pie
Posts: 12007
215
C++ Firefox Browser IntelliJ IDE Java Mac Oracle
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Prakesh,
I am not sure what you are trying to say. when you ask "how are you handling multiple threads from the calling class?".

I am trying to get confirmation that somewhere in your application you have handled multiple clients connecting and modifying the databse.
The normal way of handling this is to have set a class up as a Singleton to force all data modification requests through a single point, or to have a static lock table.
If you have not handled this, then each client connecting, will get it's own copy of the lock manager class, with it's own copy of the locks. So each client can lock the same record (according to it's private locks, no one else has (or can) lock the record it wants).
Regards, Andrew
 
Prakash Krishnamurthy
Ranch Hand
Posts: 154
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

The normal way of handling this is to have set a class up as a Singleton to force all data modification requests through a single point, or to have a static lock table.

This is what I have

Does this look okay? I dont have a singleton class as you said, but from the smoke tests that I ran, there doesn't seem to be multiple Lockmanager Instances created. But now that I think about it, maybe what you suggest is a better idea. If at all, I do create a singleton class, how can I ensure that the instance of LockManager will be the same, even if there are 1000 clients?
One thing about my design that I am not too sure about is, I dont have client ID's associated with each client. I read posts that talked about serverConnectionFactory. I am totally trusting RMI to handle that. Any comments?
[ April 12, 2003: Message edited by: Prakash Krishnamurthy ]
 
Andrew Monkhouse
author and jackaroo
Marshal Commander
Pie
Posts: 12007
215
C++ Firefox Browser IntelliJ IDE Java Mac Oracle
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Prakash,

I dont have a singleton class as you said, but from the smoke tests that I ran, there doesn't seem to be multiple Lockmanager Instances created

Consider the scenario where you have two remote clients connected simultaneously. On the server side, RMI will have created seperate threads to handle them. Each thread will look something like (this overly simplified drawing):

I think you should try it - set up two threads connectiong to your Data class, each locking a record (or the same record) and have the LockManager print the hash code of the hashTable it is locking - you should see different hash codes. If you did try to lock the same record in each thread, then you should see that both locks succeeded (which is bad).
Setting it up as a singleton would give you:

Alternatively, setting up a static HashTable would give you:

Either method should ensure that you are only using the one hashtable.
If at all, I do create a singleton class, how can I ensure that the instance of LockManager will be the same, even if there are 1000 clients?

That is the purpose of the Singleton - to make sure that the instance of the singleton will be the same. You would need to read up on Singletons before using it. But to give you an example:

Warning - I created this code from memory - it may have faults.
When you run it, you should see that the hash codes for the Single class remains
Since the constructor is private, no-one can call it to create a new copy of the Singleton.
The only way for another class to use it is to call the Singleton.getSingleton() method - the same is done if you want to get a copy of the Runtime - you have to call the Runtime.getRuntime() method.
WARNING - do not use this unless you are very happy with the concepts that it is using, and feel confident that you can explain it in the exam - you may be asked to!

I dont have client ID's associated with each client. I read posts that talked about serverConnectionFactory. I am totally trusting RMI to handle that.

This depends on how you are doing updates.
Some people here are doing server side logic - they send an update request through RMI, and on the server side the thread created does the lock, update, and unlock.
Others are doing client side logic - the client sends the lock, update, and unlock.
If you are doing the locking server side, then you can rely on the thread to act as your unique identifier.
If you are doing the locking client side, then you will have to have a unique identifier. You cannot rely on the same thread being used on the server for each of your RMI calls, so you cannot use the server thread as your identifier for the lock.
Regards, Andrew
[ April 12, 2003: Message edited by: Andrew Monkhouse ]
[ April 12, 2003: Message edited by: Andrew Monkhouse ]
[ April 12, 2003: Message edited by: Andrew Monkhouse ]
 
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic