• Post Reply
  • Bookmark Topic Watch Topic
  • New Topic

Why Is This Not Thread-Safe?

 
Aaron Dressin
Greenhorn
Posts: 29
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
PsuedoCode where appropriate:

 
Matt R. Hansen
Ranch Hand
Posts: 71
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Are you calling these methods any time you are performing an update, delete, etc...?
 
Aaron Dressin
Greenhorn
Posts: 29
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
These methods can only be called from a client proxy object through the context of the following method:
public DataInfo bookFlight(String flight, int recordNum, int numSeats)
 
Matt R. Hansen
Ranch Hand
Posts: 71
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
If I understand correctly,
Client calls bookFlight(),
bookFlight calls "update()"?
If this is the case, when are you calling lock() and unlock()? I would expect it to be called from within the bookFlight() method.
Ex:
public static bookFlight() {
lock();
update();
unlock();
}
Is this how you are doing it?
I will be out of town for a couple of days, but I will try to check in.
 
Aaron Dressin
Greenhorn
Posts: 29
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator

Also, let it be known I failed the assignment because they said 2 threads could obtain a lock on the same record. I have been unable to reproduce that error.
Thanks again
 
Matt R. Hansen
Ranch Hand
Posts: 71
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Without seeing the exact code, I am going to suggest that maybe when you are checking your key you are creating a new object. So you may be comparing 2 different Objects. For example, if you are comparing 2 Integers, try comparing their int values. Such as, Integer.parseInt(). The Values of two Integers may very well be different.
Could this be a possibility?
 
Aaron Dressin
Greenhorn
Posts: 29
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Matt,
Thanks for the response... and you make a good point. I am creating a new Integer as the key and checking the HashMap for that key. However, according to the specs, the HashMap will do an "equals()" on the Integer, which will resolve to checking the underlying primitive int values... right? This should not matter if that is correct.
Thanks,
 
Andrew Monkhouse
author and jackaroo
Marshal Commander
Pie
Posts: 12014
220
C++ Firefox Browser IntelliJ IDE Java Mac Oracle
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Aaron,
I can't see any problem with the psuedo code you posted, but the problem may not be there. The code you have posted shows how the ClientConnection interacts in a thread safe way with the RecordLockManager. Nowhere do you show what is happening in your Data class.
So - do you use the lock() and unlock() methods in the Data class?
And are your lock() and unlock() methods in the Data class thread safe?
Assume the examiner only looked at your Data class - not at any other classes. Could the examiner believe that your Data classes methods are not thread safe?
Regards, Andrew
 
Aaron Dressin
Greenhorn
Posts: 29
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Andrew... Absolutely!!! If the reviewer only looked at my Data class (and not the ClientConnection "wrapper" that is shown above) he would not only miss seeing the whole "locking" scheme, but he would not see any synchronization on the empty "lock()" "unlock()" implementations of the Data class.
Point being... my solution depends on a client using the Proxy object for client/server mode. In this scenario, there is no way for the client to call the "lock()" "unlock()" methods of the Data class directly. They must call the locking methods on the remote ClientConnection "wrapper", which uses the record lock mechanism diagrammed above.
In other words, my ClientConnection (or remote Data implementation) does not extend Data, but wraps it. I was under the impression this was reasonable with defense in my design doc.
Any more advice is greatly appreciated.
Regards,
 
Aaron Dressin
Greenhorn
Posts: 29
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Andrew, in response to your last reply in my other thread (How to dispute failure): Yes, the reviewer states that my "... locking mechanism is not thread-safe. It is possible for 2 threads to concurrently obtain a lock on the same record".
With that said, I am not sure if testing the "update" of the database as an atomic function is relevant. Regardless, the "modify" function on the data class is synchronized, so I am sure that only 1 thread my "update" the database at a time.
Thanks,
 
Andrew Monkhouse
author and jackaroo
Marshal Commander
Pie
Posts: 12014
220
C++ Firefox Browser IntelliJ IDE Java Mac Oracle
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Aaron,
Did you explain in your design decisions document that you have your locking code external to the Data class?
If you have, then this might give you something you can go back to Sun with - say that you suspect that the examiner only looked at the code in the Data class, but as explained in your design decisions document, the thread safe locking code is in classes 'x' and 'y'.
I agree that my comment about testing the "modify" routines is not relevant given the circumstances - just covering my bases .
Regards, Andrew
 
Terry Martinson
Ranch Hand
Posts: 293
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Aaron - can you please continue to keep us posted on how this turns out for you? Good luck and I hope they change their decision for you.
TJ
 
Aaron Dressin
Greenhorn
Posts: 29
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Andrew,
Thanks for your reply. I will need to consult my design document to see whether I specified this design choice or not. I have a question about that however: Are you saying that failing to document this choice (even though a thorough examination of the code would clearly show this) could justify Suns choice to deduct over 30 points from me?
Thanks,
Aaron
 
Andrew Monkhouse
author and jackaroo
Marshal Commander
Pie
Posts: 12014
220
C++ Firefox Browser IntelliJ IDE Java Mac Oracle
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Aaron,
Unfortunately, yes this could cause failure.
In the section "Extending suncertify.db.Data" the instructions state that "Record locking must be implemented using the methods public void lock(int) and public void unlock(int).". So not implementing anything in the Data class's lock() and unlock() method (or the subclass of Data class) and not explaining your reasoning could result in a failure.
Consider it from the perspective of the junior programmer who has to maintain your program: they have one set of documents telling them that locking is implemented in the Data class, but when they go there they cannot see any code. So they would have to try and find which class(es) do the locking, and then they can try and work out how these other classes really work. And that is always assuming that they are bright enough and motivated enough to do this: they could just go and complain to the boss that they are going to need 3 times longer to look at the locking code than the boss intended them (in which case the boss is going to tell you off).
Same thing happens with the review of this assignment: the examiner has allocated a certain amount of time to looking at your locking code and ensuring that it is thread safe. If they have to search for it, then this can throw out their entire schedule. They may (unfortunately) take the easier (and more lucrative for them) stance that the Data class's lock() method is not thread safe and you haven't given them instructions on where to look for the safe code, so they fail you.
Note: I am not saying that this is what happened: only that it could be.
Regards, Andrew
 
John Smith
Ranch Hand
Posts: 2937
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Aaron,
The code that you posted looks thread-safe, assuming that the usage of it is also thread safe in respect to class Data and the underlying database file. One possibility where I see you may not have ensured the thread safety is the Data class itself. Let me ask you this: did you choose to create only one instance of Data, shared among all the clients, or do you have multiple instances of Data, one per client? If it is the latter, you may have a problem, depending on how your implemented the handling of the reading and writing to the database.
 
Aaron Dressin
Greenhorn
Posts: 29
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thanks everyone for your help. Eugene, my solution was not the latter: I only have 1 static data for all remote clients.
I have contacted Sun regarding this and have received a response requesting a full description of my dispute to forward to the head grader. Wish me luck! I will keep you all posted on what happens.
Assuming I failed because of what Andrew and I discussed above, I atleast would like clarification on that... so that I do not try to re-implement my whole locking scheme... rather than just providing some supplementary documentation.
Regards,
Aaron
 
Aaron Dressin
Greenhorn
Posts: 29
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
All,
It is official!!! I disputed my failure and they changed my grade to 144!!! I am very happy that my grade was changed... but I am still concerned that I failed in the first place. My code was correct the whole time and my design decisions were documented in my design document.
Thanks to all who helped me with this!
-Aaron
 
George Marinkovich
Ranch Hand
Posts: 619
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Hi Aaron,
Congratulations on passing your SCJD.
Yet another example that perseverence pays. Thanks for posting your story, you may have inspired others similarly situated.
 
Philippe Maquet
Bartender
Posts: 1872
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Congratulations, Aaron!
Regards,
Phil.
 
Andrew Monkhouse
author and jackaroo
Marshal Commander
Pie
Posts: 12014
220
C++ Firefox Browser IntelliJ IDE Java Mac Oracle
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Everyone,
Aaron also has this thread in the Sun Certification Results forum. Please add your congratulatory notes there.
I have left this thread open for now in case anyone wanted to discuss the failure / potential for failure.
Regards, Andrew
 
  • Post Reply
  • Bookmark Topic Watch Topic
  • New Topic