This week's book giveaway is in the OCAJP 8 forum. We're giving away four copies of OCA Java SE 8 Programmer I Study Guide and have Edward Finegan & Robert Liguori on-line! See this thread for details.
I've read a lot of topics about the locking mechanism and just want to get your opinion on whether my strategy holds up?
1.Lock and unlock method are not synchronized - all other methods in the class are synchronized
2.Uses reentrant lock to create critical section for threads looking for record lock
3.Once thread has reentrant lock it calls a synchronized method to check if record exists
4.Waits on a reentrant lock Condition object inside while loop if record lock held by another thread
5.When record lock is available thread calls synchronized doesRecordExist method again to verify record was not deleted
6.Creates cookie and adds to map
7.Releases reentrant lock and exits lock method
With this approach it is possible that thread A can execute one of my public synchronized methods while Thread B is inside the lock or unlock method. However because the method to check if a record exists is synchronized it means that the lock method will have to wait for any CRUD operations to complete before it can check if record exists. So no data corruption in my opinion.
To lock the database I then have a synchronized method which closes the RandomAccessFile object in my DataFileAccess class (this class does all IO operations on data file) and then set its reference to null. Because the method is synchronized it means that no other CRUD operation can occur at this point. It is possible that another running thread could enter the lock method after this and attempt to obtain a lock for a record however an IllegalStateException will be thrown when the check is called from the lock method to see if a record exists or not. It is possible also that another thread who was waiting to enter a synchronized block to update or delete a record does so but an IllegalStateException will be thrown because the DataFileAccess reference is no longer valid.
The 1st question that comes into my mind: you add extra complexity to the Data class by using another technique for making lock/unlock methods handle multiple requests. What's the benefit of using this technique? You could make it a lot simpler by marking every method synchronized and that would be quiet normal because you made all crud-operations synchronized. So why follow another approach for the lock/unlock methods
it was a choice between synchronizing on my singleton class or using the lock API. I choose the lock API which ment I could no longer synchronize the lock or unlock methods. I use only the basic features of the API so not a lot of benefit. In the future the way threads release locks could be enhanced by using separate lock conditions for each thread but i haven't implemented this and only mentioned as a possible future enhancement. Looking at it now, the way i have it implemented does not offer anything additional so i will consider reverting to just keeping all methods synchronzed and then just synchronize on the Data instance in my lock and unlock methods.
Joined: Feb 23, 2008
when the application shuts down should i be waiting for any existing threads , which hold record locks, to finish before i persist my record cache to the db file and close the RAF instance? I could force the shutdown method to wait on the same lock as the threads in the lock method until my lock map is empty and then shutdown?
As long as you don't end up with a corrupted database file (closing when some record is being updated/created), you'll be fine. I didn't wait until the map with locked records was empty. When the shutdown hook gets CPU time, it's game over for all other waiting threads