aspose file tools*
The moose likes Developer Certification (SCJD/OCMJD) and the fly likes How to cope with situations that should never occur? Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login
JavaRanch » Java Forums » Certification » Developer Certification (SCJD/OCMJD)
Bookmark "How to cope with situations that should never occur?" Watch "How to cope with situations that should never occur?" New topic
Author

How to cope with situations that should never occur?

Roel De Nijs
Bartender

Joined: Jul 19, 2004
Posts: 5545
    
  13

Hi all,

Let me show with some pseudo-code what i mean (just posting the relevant code). For booking a room with recNo 1 i have following code:



also my unlock-method throws a RecordNotFoundException which i have to catch (but again that exception should never occur because lock method would have wrong implementation.

so should i throw a self-created runtime exception, or maybe a IllegalStateException? or what is your opinion about this matter?

Kind regards,
Roel


SCJA, SCJP (1.4 | 5.0 | 6.0), SCJD
http://www.javaroe.be/
Alecsandru Cocarla
Ranch Hand

Joined: Feb 29, 2008
Posts: 158
In my implementation, Data's lock() only checks if the record is valid before the actual LockManager's lock(). It does not make another check after locking. So it's possible to lock a deleted record. That's why update() and delete() can still throw RecordNotFoundException.


SCJP 1.4 100%
SCJD 99.5%
Roel De Nijs
Bartender

Joined: Jul 19, 2004
Posts: 5545
    
  13

Hi Alecsandru,

I see that in your implementation it's possible to lock a deleted record because the check if record is valid and the lockmanager.lock call are not 1 atomic operation.

In my implementation all methods are marked synchronized, so when lock is called and record is valid a RNFE being thrown by read/update/unlock could never happen. So i'm not quiet sure how to handle this, have some possibilities:
a) creating own exception and wrapping the RNFE in it
b) using an IllegalStateException
c) using an assert

For the moment i would choose a or b, with a small advantage for b. But maybe someone has a better suggestion.

Regards,
Roel
Alecsandru Cocarla
Ranch Hand

Joined: Feb 29, 2008
Posts: 158
I see that in your implementation it's possible to lock a deleted record because the check if record is valid and the lockmanager.lock call are not 1 atomic operation.

Do you have a Data singleton? Otherwise, there would be no use in having synchronized methods. If yes, then this means your locking mechanism (the lock manager) is useless, since anyway lock/update/unlock are already thread safe. I don't really see why you would synchronize your methods, since lock() is supposed to take care of this. Which methods exactly do you have synchronized?
Roel De Nijs
Bartender

Joined: Jul 19, 2004
Posts: 5545
    
  13

Alecsandru Cocarla wrote:Do you have a Data singleton?

Yes, I have

Alecsandru Cocarla wrote:Which methods exactly do you have synchronized?

Every method

Alecsandru Cocarla wrote:If yes, then this means your locking mechanism (the lock manager) is useless, since anyway lock/update/unlock are already thread safe. I don't really see why you would synchronize your methods, since lock() is supposed to take care of this.

Totally don't agree with you. the locking mechanism just prevents situations like this (and that has nothing to do with how i made my data-class thread-safe):
- Client A checks that record 5 is available
- Client B checks that record 5 is available
- Client A updates record 5 with their client number
- Client B updates record 5 with their client number
Alecsandru Cocarla
Ranch Hand

Joined: Feb 29, 2008
Posts: 158
Ok, then, inside lock() of your lock manager, what mutex are you waiting on? It should be the Data singleton instance, otherwise you're exposed to deadlocks. If it's the Data singleton, then I'd say it's not a nice design to have your lock manager depend on Data, and Data on the lock manager. But ok, this is just a design decision, and the code should work correctly.

If your lock() and unlock() methods are exposed remotely, then I say it is still possible that you get a RecordNotFoundException inside your update() method, even if your methods are synchronized.
Roel De Nijs
Bartender

Joined: Jul 19, 2004
Posts: 5545
    
  13

my mutex is idd the data-singleton and i have no seperate lockmanager-class

my lock/unlock methods are not exposed remotely. only a few business methods are exposed
Alecsandru Cocarla
Ranch Hand

Joined: Feb 29, 2008
Posts: 158
As I said, in this case your locking functionality is useless, since there is no possibility that two clients ever do something like:


Since they're synchronized, the operations performed by A and B will always be performed sequentially:


This is perfectly equivalent to:


This is because B cannot even enter the book() method until A finishes its book().
Also, this approach will substantially diminish concurrency. No client will be able to create/read/update/delete anything while another one is inside one of Data's methods.
Roel De Nijs
Bartender

Joined: Jul 19, 2004
Posts: 5545
    
  13

Hi Alecsandru,

I've used Roberto Perillo's test-case and if i check the output i have quiet a lot of lines in the output telling me that threadX is waiting for lock on record 1.

i have a data-class (singleton with record-cache) with lock/update/find/read/delete/create/unlock/isLocked and every method is synchronized (to be very clear: my business methods, like bookRoom are no part of the Data-class, but are in a seperate Service-implementation. this impl has a reference to the Data-class of course)

if i use Roberto's test-case as an example, then i have following code:


so if i run a test-case based on this class with 2 threads trying to update record 1:
- threadA calls data.lock(1);
// in the lock-method a check is done to see if record 1 is valid (recNo exists and isn't locked) --> it is, so record 1 is locked by threadA
// now threadB swoops in
- threadB calls data.lock(1);
// in the lock-method a check is done to see if record 1 is valid (recNo exists and isn't locked) --> it is not because already locked, so threadB waits
// and here is threadA again
- threadA calls data.update(1, newRoomData);
// record is updated
// (if threadC or maybe even a threadD would exist and they unlock their locked record, every thread is notified so it is possible that threadB gets a turn to have some cpu and will go back in wait-state because record 1 is still locked.
- threadA calls data.unlock(1);
// record is unlocked and all waiting threads are notified. threadB gets the cpu and finishes his program

if i leave the locking out (as you suggests) in this little program that wouldn't make a difference indeed, because they will both update their record (in a threadsafe way) and that's it. but if the program is adapted with a check to see if the record is available or not, the locking must not be left out or you risk to end up with multiple clients booking the same room.

and you are certainly right in your last point: concurreny is not that big, but it is a very easy approach and very understandable for a junior programmer. and that's what i will mention in my decisions document. also there is no request to be as concurrent/performant as possible
Alecsandru Cocarla
Ranch Hand

Joined: Feb 29, 2008
Posts: 158
Aaaaah, I thought your business methods are part of Data... You should have mentioned that from the start

This is what made me think that the code from the first post is inside your Data:
In my implementation all methods are marked synchronized, so when lock is called and record is valid a RNFE being thrown by read/update/unlock could never happen.

From here, I (mis)understood that the code you posted is also synchronized and inside Data.

Now, since your Data is also your lock manager, I understand that when the thread wakes up it will check again if the record is still valid, and won't lock unless it's valid (that's why you don't have to call unlock()).

I'd say IllegalStateException. I wouldn't create a new exception just for wrapping an exception that should never occur. As for assertion, it might also be appropriate, just that in these complex situations, involving threads, I wouldn't trust an assertion. And it's against the principle of "never swallow and exception".

Or... you could disable the re-check inside lock() and let update() check for itself, and you don't have to worry about this issue anymore (Anyway, you'll have to wonder "why did SUN plant this Exception in my interface if it should never be thrown?")

And, again, sorry for the misunderstanding... It's just that I'm not used to giving answers without first seeing if the problem is real or not (or if there are no other problems instead), and only afterwards answering (if there's anything left to answer).
Roel De Nijs
Bartender

Joined: Jul 19, 2004
Posts: 5545
    
  13

Alecsandru Cocarla wrote:
And, again, sorry for the misunderstanding...


Not a problem. i had to make clear from the beginning that it was in a seperate business service (instead of thinking it was obvious for everyone). I just added clarification in my last post because it seemed a bit weird that you scored 398/400 and not get this approach

depending on the given interface of sun you have to call lock before you can call update/delete, so if the record is not valid i will notice it on the lock-method and thus that's the one method that should throw the RNFE (besides read-method of course). so i could also override (in my own interface extending from sun's) the update/delete/unlock and not have RNFE in method signature. That would solve some issues but not all of them, because the call to read still throw's the impossible RNFE

another thing i could do is:

this seems also to be a reasonable approach (except for the double data.unlock)
Alecsandru Cocarla
Ranch Hand

Joined: Feb 29, 2008
Posts: 158
I'd rather have the first post approach, since it's clearer where the exception comes from, which one is possible and which one is not possible.
And no finally block where to call unlock... I don't really like this either.

 
It is sorta covered in the JavaRanch Style Guide.
 
subject: How to cope with situations that should never occur?