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
