This week's book giveaway is in the OO, Patterns, UML and Refactoring forum. We're giving away four copies of Refactoring for Software Design Smells: Managing Technical Debt and have Girish Suryanarayana, Ganesh Samarthyam & Tushar Sharma on-line! See this thread for details.
I'm currently having an issue with running multiple-threaded access for the delete method of assignment. The method works say 3 out of 4 times and then can enter deadlock. I will post code in hope someone spots a problem. I have 2 caches a valid and deleted record numbers cache. Below is my lock method.
This is probably fairly standard.
in delete method, just check is the record number passed locked by passed cookie
Anyone spot anything in my logic, that could be wrong? Any suggestions are very welcome, as one can imagine, multi-threaded issues are difficult to resolve!
Why don't make it a checkRecNo method which will throw RNFE when recNo is invalid. A lot less lines of code and code would become a lot easier to read (in my opinion).
I'm completely not familiar with the new locking api (used wait/notifyAll/synchronized), but it seems the isRecordNumberValid is a static method because RoomReservationFileAccess seems a class (if it is an object name, it should not start with capital letter). Are you sure these static methods are thread-safe too?
I guess with your logic is nothing wrong, because in my application I used similar logic:
a) check recNo
b) wait (if record is already locked)
c) check recNo
d) lock record by adding recNo to a map
Solved this issue. I was calling notify() instead of notifyAll() on the condition variable when unlocking. Damn, that was difficult to detect. Have take Roel's comments on board, and was already throwing the RNFE exception from the isRecordNumberValid function, just I'm new to Exception handling so I was creating unneccessary code in the lock function. Alls well again for Halloween time! Can go trick or treating!
Andrew Monkhouse wrote:When I see that they are taking an active interest in a topic I usually feel that there is little left for me to add.
Honestly, I feel really honored to read this. Andrew, thanks for the words. It feels really good to know that we are able to help people, and it's also fun! We'll continue to help people, and I'm sure my good buddy Roel also thinks like me!
the first call of RoomReservationFileAccess.checkRecNo(recNo); is it really necessary?
If the record is deleted no one has the ability to change it (or to lock it), if it doesn't exist there's no reason to wait for lock... So, why to do the first check?
perhaps you have already noticed that i have the same locking strategy within my locking manager...
So, one follow-up question:
Due to safety reasons i have to make my check-record-no method thread-safe too....but, i was wondering if a read lock is enough or if i have to use a
write lock when checking it (it == my map storing record information)? What are your thoughts?
How did you handle the InteruptedException finally? I saw some post from you where you wrote that you are using
instead of wrapping it up into some user-defined RuntimeException, right? Was that your final decision or did you change before submitting to SUN?
According to your 1st question: In my assignment I didn't use read and write locks, just synchronized the complete methods, so I'm not an expert in this matter. I would think a read lock is just fine, because check-record-no method would in my opinion just need to read to your map (and not write).
This thread is not about InterruptedException and how to handle it, so I'm not going to answer this question here because we should try to stick as much as possible to "1 thread for 1 question"-policy. So it becomes for other people trying to look for answers a lot easier to find what they are looking for. So feel free to post your question in a new thread or in the thread I started.