Ziji (Jay) Zhang

I guess the difference is that
if you use synchronized(this), you lock every thing in this object.

But if you use:
synchronized(lock){}, you can also specify what you want to be synchronized, only a member of this object, or the whole object.

So you can achieve synchronized(this) with synchronized(lock), but it is not true the other way around,
synchronized(lock){ this.counter ++; /* only manipulate one member varaible */ }.

In this case, all other member variables can still be accessed in non-synchronized method/block.


Ziji

I guess the difference is that
if you use synchronized(this), you lock every thing in this object.

But if you use:
synchronized(lock){}, you can also specify what you want to be synchronized, only a member of this object, or the whole object.

So you can achieve synchronized(this) with synchronized(lock), but it is not true the other way around,
synchronized(lock){ this.counter ++; /* only manipulate one member varaible */ }.

In this case, all other member variables can still be accessed in non-synchronized method/block.


Ziji

Thanks for the comments.


Object lock = new Object(); ... public boolean isLocked(int recNo) throws RecordNotFoundException { synchronized (lock){ return lockedRecord.containsKey(recNo) ; } } , which is exactly the same as synchronized (this).

I read the Java tutorial,

It says that syncronized on this and synchronized on a member lock of this
are not equivalent. See the link:

http://java.sun.com/docs/books/tutorial/essential/concurrency/locksync.html

Ziji

I agree with Gabriel,

and the most important reason for me is the object locked is the record and not makes sense to unlock a thing than doesn't exists. (I see a deja vu about this point )

I thing to clarify, Should we check isRecordLock() in the delete
method body of the Data class?

I am doing a check before deleting the record from database file.
then invoke lockManager's unlock method (no Data class's unlock )

Ziji

I agree with Gabriel,

and the most important reason for me is the object locked is the record and not makes sense to unlock a thing than doesn't exists. (I see a deja vu about this point )

I thing to clarify, Should we check isRecordLock() in the delete
method body of the Data class?

I am doing a check before deleting the record from database file.
then invoke lockManager's unlock method (no Data class's unlock )

Ziji

Marcelo,

I think you pretty much figured all out.

There are a couple of things I saw in your unlock method:
DBAccess interface signature: public void unlock(long recNo, long cookie) throws SecurityException; My implementation of this interface: public void unlock(long recNo, long cookie) throws IllegalArgumentException, SecurityException { try { /* check if has the logical record lock */ logicalRecordLock.logicallyLocked(recNo, cookie); /* release the logical record lock */ logicalRecordLock.unlock(recNo, cookie); } catch (IllegalArgumentException iae) { /* re-throw the exception */ throw iae; } catch (SecurityException se) { /* the record is not locked, then re-throw the exception */ throw se; } }

Don't you compare cookie/owner of the lock with the current thread?

within your try block, shouldn't you have a if clause:

if (record is locked){
unlock record;
}


Ziji

Marcelo,

I think you pretty much figured all out.

There are a couple of things I saw in your unlock method:
DBAccess interface signature: public void unlock(long recNo, long cookie) throws SecurityException; My implementation of this interface: public void unlock(long recNo, long cookie) throws IllegalArgumentException, SecurityException { try { /* check if has the logical record lock */ logicalRecordLock.logicallyLocked(recNo, cookie); /* release the logical record lock */ logicalRecordLock.unlock(recNo, cookie); } catch (IllegalArgumentException iae) { /* re-throw the exception */ throw iae; } catch (SecurityException se) { /* the record is not locked, then re-throw the exception */ throw se; } }

Don't you compare cookie/owner of the lock with the current thread?

within your try block, shouldn't you have a if clause:

if (record is locked){
unlock record;
}


Ziji

Marcelo,

I think you pretty much figured all out.

There are a couple of things I saw in your unlock method:
DBAccess interface signature: public void unlock(long recNo, long cookie) throws SecurityException; My implementation of this interface: public void unlock(long recNo, long cookie) throws IllegalArgumentException, SecurityException { try { /* check if has the logical record lock */ logicalRecordLock.logicallyLocked(recNo, cookie); /* release the logical record lock */ logicalRecordLock.unlock(recNo, cookie); } catch (IllegalArgumentException iae) { /* re-throw the exception */ throw iae; } catch (SecurityException se) { /* the record is not locked, then re-throw the exception */ throw se; } }

Don't you compare cookie/owner of the lock with the current thread?

within your try block, shouldn't you have a if clause:

if (record is locked){
unlock record;
}


Ziji

Gabriel,
Thanks for your explanation.
It is more clear to me now.
I am used to use intrisic lock mechanism.


ziji

Hi,
I have been searching for topics regarding the usage of synchronized and reentrantLock but had no luck.
In my project, I followed the Andrew's book, using ReentrantLock, as it
is more object oriented, and you can also upgrade to hand-over-hand locking.
But I still have a question that I cannot answer:
For example, for this method using ReentrantLock:
public boolean isLocked(int recNo) throws RecordNotFoundException { lock.lock(); try{ return lockedRecord.containsKey(recNo) ; }finally{ lock.unlock(); } }

where lock is an instance of ReentrantLock decared as member of lockManager class.

Is it equivalent to:

public boolean isLocked(int recNo) throws RecordNotFoundException { Object lock = new Object(); synchronized (lock){ return lockedRecord.containsKey(recNo) ; } }

or to :

CODE]
public boolean isLocked(int recNo) throws RecordNotFoundException {
synchronized (this){
return lockedRecord.containsKey(recNo) ;
}
}
[/CODE]

???
Can any one shed some light on this?

Ziji

Joaquim,

lock->delete record->unlock sequence

This is one issue I had to spend two cups of coffee to think about. I did not keep locking and persistance classes separated, therefore one could not lock or unlock a deleted record. Since deleting a record requires "owning" its lock, I decided unlocking is not required after deleting one record, for simplicity reasons.

I have been reading posts about this issue for a while,
If you don't release the deleted recNo, you will have trouble later,
For example, If you want to re-use that recNo for creating new record,
then that recNo will go back to normal, and you can never lock that record by other thread again, as it is locked by a thread during deleting.


Ziji

Joaquim,

lock->delete record->unlock sequence

This is one issue I had to spend two cups of coffee to think about. I did not keep locking and persistance classes separated, therefore one could not lock or unlock a deleted record. Since deleting a record requires "owning" its lock, I decided unlocking is not required after deleting one record, for simplicity reasons.

I have been reading posts about this issue for a while,
If you don't release the deleted recNo, you will have trouble later,
For example, If you want to re-use that recNo for creating new record,
then that recNo will go back to normal, and you can never lock that record by other thread again, as it is locked by a thread during deleting.


Ziji

OK,

1. During this sequence: lock->delete record->unlock, if I check the db.isValidRecord() inside the call to locking.unlock(), as you mention, the record will not exist anymore, and an RNFE will be thrown. But its not true, because I need to unlock the record... How did you handle that?

2.About InterruptedException. I did not understand it yet. My implementation is swallowing this exception and checking again the while-expression, and if it was not reached, sleep again. Is this wrong? I need to interrupt the process and return some exception to the client?

I have read many posts here, Here is the approach:

1. if you delete a record, then you shouldn't call unlock() method in data class, instead, you should release the deleted recNo your self,
but this approach broke the standard sequence of : lock... some operation ... unlock.
2. the second approach is in lock and unlock, you don't care if recNo is
deleted or not, so lock and unlock can also operate on deleted record,
only when recNo is not lock or not in range, you throw RNFE.

About interruptedException, I just throw RNFE,
May be you and others are right, probably, we should wrap it up with a
RuntimeException, you cannot just throw InterruptedException, as the Sun's
instruction doesn't declare this Exception being thrown, you will lose points if you throw it.

Ziji

Shouldn't checking record exists and calling dataManager.create(),
be in the synchronized block?
Other thread may change the data between these two calls.
Ziji

In the Data class I call this method,
but I also check if record is valid or not first,

public boolean isLocked(int recNo) throws RecordNotFoundException { if ( ! db.isValid(recNo)){ throw new RecordNotFoundException("Record with recNo : " + recNo + " doesn't exit in the database"); } return lockMgr.isLocked(recNo); }[

Thanks,
Ziji