In the supplied code, almost all methods are synchronized except writeRecord which I wonder why ? I have following questions : 1)Instead of synchronizing so many methods why not just synchronize writeRecord and seek ? because these are the two methods where real update of data and moving file pointer happens. 2) If modify and getRecord are already synchronized, why lock and unlock methods are even needed let alone synchronizing them ? 3) I wrote a driver program (which creates threads) to test my lock, unlock. Surprisingly there is no change in program output if I comment out lock and unlock methods, both in local and remote mode. I am using RMI and there is only one instance if remote server in remote mode, in local mode there are as many instances of Data class as there are clients. Are lock unlock methods really any useful ? Can anybody clarify ? thanks in advance.
1) Because all of the other operations are actually composite operations, which will go wrong if the database changes while the operation is under way. Let me give a simple example:This code is threadsafe, because Vector is threadsafe, right? Wrong! Between the call to v.size() and v.get(i), another thread may remove an element from the Vector. If i happened to point to the very last element of v, you'd get a nice ArrayIndexOutOfBoundsException. Of course, the devious thing is that you might not see this during development or testing. With this in mind, have a careful look at Data again. You will find a dozen places where this type of thing might happen (and you'll miss two dozen others). 2) Please carefully distinguish between synchronization and record locking. They work at completely different levels. Synchronization works on Data as a whole to preserve the integrity of the object. Locking works on individual records to ensure that only one client can book a seat at the same time (otherwise, race conditions similar to that outlined under 1 can cause a seat to be booked twice). 3) Write a test client in remote mode that doesRun two or three copies. Watch them get in each other's way and double-book seats. Now add locking until this no longer happens. Of course, in real life the client doesn't sleep(). But that merely reduces the chance of race conditions like the above, not eliminate it. - Peter
posted 17 years ago
Peter, Thanks very much for your detailed response to my questions. I have following comments to make 1) I agree to your point, that any method that does multiple operations and if there is a need to treat all operations as one transaction, the method needs to be synchronized and that is why methods like getRecord, add, modify etc are synchronized. It also seems convincing to me that writeRecord does need to be synchronized (inspite of the fact that actual write into file happpes here) because all methods that call writeRecord themselves are synchronized and second (more) important reason is writeRecord is a private method, so nobody else outside the Data class can directly call this method. But on same token, why readRecord and seek methods are synchronized ? these methods are private too. If the methods that call private methods are themselves synchronized, what is the need of making private methods synchronized ? why treat writeRecord differently by not declaring it synchronized when readRecord and seek are declared synchronized ? 2) Again here, comments as above about treating multiple operations as one transaction apply. I guess I am not seeing the effect of locking in my test program (as you pointed out) because I have not introduced a deliberate delay like sleep. I am going to try adding a delay and then observe the behaviour with and without lock. BTW, I have a new question now, of tracking client id. I think, my code ensures that there is no more than one lock on a given record at any given time i.e a thread seeking to lock a record already locked by another thread has to wait. The only way a subsequent thread can obtain a lock on same record is only when first thread unlocks the record. So, I do not really see a need to track thread id. I may be wrong here. Can anybody comment ?? Thanks again
Peter den Haan
posted 17 years ago
In my copy of Data, seek() is indeed unnecessarily synchronized. It is private and only called from synchronized methods, and I cannot see any reason why this would ever not be the case. The writeRecord() method was not synchronized and didn't need to be for the very same reason. So I agree with you. As long as there is no conceivable reason why these private methods would ever be called from a non-synchronized method, there is no reason whatsoever to synchronize them. Regarding the client ID, in my assignment there was an explicit requirement that a duplicate call to lock() should have no ill effect. This can be fulfilled in two ways: (1) you associate each lock with something identifying the client (in RMI, don't use the Thread!) (2) you have a client-side adapter that keeps track of locks held by that client and simply suppresses any duplicate calls. I should add that some simply ignore this requirement and still pass, albeit with lower marks. - Peter