Urgent: Need suggestion on my Lock/UnLock

Hi all
I have designed my lock and unlock in this manner.
I am getting wrong results when scheduled three
threads that does the folowing sequence
lock,read,modify,unlock. 100 times.
Can you guys suggest me if i missed any thing. or what
is wrong in this lock and unlock implementation.
public void lock(int recno)
{
Thread currentThread = Thread.currentThread();
synchronized(lockedRecords)
{
Integer recordToBeLocked = new
Integer(recno);

if(lockedRecords.containsKey(recordToBeLocked) )
{
try
{
lockedRecords.wait();
}catch(InterruptedException e)
{}
}
else
{

lockedRecords.put(recordToBeLocked,currentThread);
}
lockedRecords.notifyAll();
}
}

public void unlock(int recno)
{
Thread calingThread = Thread.currentThread();
synchronized(lockedRecords)
{
Integer recordToBeUnlocked =new
Integer(recno);

if(lockedRecords.containsKey(recordToBeUnlocked) &&

calingThread.equals((Thread)lockedRecords.get(recordToBeUnlocked)))
{

lockedRecords.remove(recordToBeUnlocked);
}
lockedRecords.notifyAll();
}
}
Regards
Reddy.

In the first place, I'm not quite sure I'd use the current thread to identify each client... In my opinion it is not a good practice, and it certainly relies on the fact that the thread that calls each method will be the same "thread object", which I personally dislike....
I believe the trouble with your implementation is in the piece of code where the thread waits to be notified....
public void lock(int recno)
{
Thread currentThread = Thread.currentThread();
synchronized(lockedRecords)
{
Integer recordToBeLocked = new
Integer(recno);

public void lock(int recno)
{
Thread currentThread = Thread.currentThread();
synchronized(lockedRecords)
{
Integer recordToBeLocked = new
Integer(recno);

// NOTICE THAT IF A CLIENT WANTS TO LOCK A RECORD ALREADY LOCKED BY ANOTHER CLIENT, IT WILL ENTER
// THE IF BLOCK, BUT IT WILL NEVER SET THE RECORD AS LOCKED (THE ELSE
// BLOCK IS NEVER VISITED!!!)
if(lockedRecords.containsKey(recordToBeLocked) )
{
try
{
// ANOTHER PAINFUL BUG IS JUST TESTING FOR THE CONTAINSKEY CONDITION AND WAITING ON AN IF... AS YOU ARE USING NOTIFYALL, YOU SHOULD DO THIS IN A WHILE LOOP, AS MANY THREADS WILL BE WOKEN UP, AND SOME WILL HAVE TO KEEP ON WAITING...
lockedRecords.wait();
}catch(InterruptedException e)
{}
}
else
{
lockedRecords.put(recordToBeLocked,currentThread);
}
lockedRecords.notifyAll();
}
}
My solution to your troubles should be something like:
public void lock(int recno)
{
Thread currentThread = Thread.currentThread();
Integer recordToBeLocked = new Integer(recno);
synchronized( lockedRecords )
{
try
{
// YOU SHOULD ALSO CHECK HERE FOR THE DB LOCK
while( lockedRecords.containsKey(recordToBeLocked) )
lockedRecords.wait();

lockedRecords.put(recordToBeLocked,currentThread);
}
catch( InterruptedException e )
{
lockedRecords.notifyAll();
throw new IOException( e.getMessage() );
}
}
}
Notice that I tried to mimic your piece of code, and hence, I used the current thread and didn't check for the global DB lock (something you should do).
Hope this helps!!!
Benjam�n

Just an additional note .... if you are using rmi then the spec does not guarentee that the client request will execute in the same thread for each request ....
Conor