In some older threads Andrew and a few others mentioned breaking up unlock. Sounds like a great idea. But I have a few questions regarding this. My break down would look like:
If I synchronize on my collection lockedCntrRecords, should I synchronize on that collection within each of the methods? Or would it just be easier to synchronize the different methods? Or is it just a matter of opinion? Example
If I synchronize on my collection lockedCntrRecords, should I synchronize on that collection within each of the methods?
No way! You first sample of the unlock() looks great is valid. You have to guarantee atomicity of the unlock() transaction. If you synchronizie each submethod separately no atomicity of the unlock() will be guaranteed. Best, Vlad
Joined: Mar 21, 2003
Thanks Vlad, but this raises some questions.
No way! You first sample of the unlock() looks great is valid. You have to guarantee atomicity of the unlock() transaction. If you synchronizie each submethod separately no atomicity of the unlock() will be guaranteed.
So in other words forget any synchronization on the other three methods. OK, but can you explain to me how that breaks atomicity rather than keeps it? And remember I will be calling unlockRemoveFromCollection(int recordNumber) from my delete method or any method that throws RecordNotFoundException (Which is the point of breaking the unlock method up in the first place). Must I not then own the lock on lockedCntrRecords in the method to call notifyAll()? Am I lost?
Hi Bill, Surprisingly I disagree with Vlad on this one: I don't think you need atomicity for those three operations as a whole. Taking each of them in turn:
unlockValidateRecordExists(recordNumber) If, after validating the record exists, you loose the mutex on lockedCntrRecords, and someone deletes the record, so what? The current thread will just continue as though the record was valid, and it will then fail on the next test (since for someone else to delete the record, they must own the lock). So the results will be the same as if you had guaranteed atomicity around the three method calls.
unlockValidateSecurity(recordNumber,cookie) Again this should not cause us a problem. If you loose the mutex on lockedCntrRecords after this call, you will still own the lock on the record so when you regain the mutex nothing will have changed.
unlockRemoveFromCollection(recordNumber) Nothing to say.
As for moving the synchronization into those methods: As you have noted, you will need to own the mutex on lockedCntrRecords in order to call notifyAll(), so this is a good thing. Regards, Andrew
Thanks Andrew, I was hoping you would reply. I have a question though. The main reason I am doing this is so I can call unlockRemoveFromCollection(recordNumber) from other methods before they throw an exception. Would it be cleaner to just leave my original unlock as is and create another unlock method that takes just the record number and looks something like:
and thats it.
Joined: Mar 21, 2003
somehow I posted by accident. I wanted to add that even if this.lockedCntrRecords.remove(String.valueOf(recordNumber)) returns false so what. But maybe then I should not call notifyAll.
Joined: Jul 07, 2003
Hi Bill and Andrew, Andrew:
If, after validating the record exists, you loose the mutex on lockedCntrRecords, and someone deletes the record, so what?
Ups. I agree. Honestly saying I thought about lock() method. It is much more tricky there. Regarding unlock() I agree with Andrew and recognize my mistake in my previous mail. Sorry. Best, Vlad
author and jackaroo
Hi Bill, I think it is better to only have one method which removes the record number from the collection - having it in two places invites problems. And I would only call notifyAll() if you actually removed a record from the collection. Regards, Andrew