Record Locking Test

[Nandini Sriram]

Hai,
I have done the testing for lock and unlock using a Test case of about 20 threads, each booking upto 10 tickets. There seems to be no problem in modification. Is this enough, or do I need to check anything else to ensure my record locking works properly.
Thanks
Nandini

[Jim Yingst]

Well, what requirements have you been given? What happens if one client locks a record, and then another client tries to lock the same record? Or, what if the second client tries to do an update(), delete() or unlock() without locking? And for all of these - what happens when the first client eventually does unlock the record? Does the second client experience the expected behavior? What about a third client attempting to lock the record after it was released?

[Qusay Jaafar]

You need to test lock() and unlock() method.
I think your test was about bookingFlight() method.
What about this Senario
did you handle it?

[Nandini Sriram]

Hi Jim and Qusay,
Thanks for replying.
In my test case, I run 20 threads that attempt to book (lock-book-unlock) the same flight (each booking from 1-10 tickets). When the first client acquires the lock, the rest are made to wait. Then after the record is unlocked, the next thread is allowed to lock. So after every unlock only the next thread is given the lock.
When I make a client to unlock without locking, I'm able to see the message
"Record not available for unlocking".
I have a doubt in one of the assignment requirements -
If an attempt is made to unlock a record that has not been locked by this connection, then no action is to be taken.
My doubt is this - "no action to be taken" means the client's booking itself (lock - book - unlock) must be invalidated OR the booking allowed but ignore the call to unlock?
And Qusay, I will try out your scenario and let you know.
Thanks

[Nandini Sriram]

Hai Qusay,
I tried out this scenario (is this the one you had mentioned?)
A locks #13, followed by B,C, and D.
... 'A' does not unlock ...
... B C D are made to wait ...
... after about 10 minutes "unreferenced" called... locked record is unlocked ...
... The next thread gains the lock and the last 2 are put to wait...
Here, I expected the threads to go quickly since the last 3 threads do the locking and unlocking properly. But there seems to be a long wait, even after the next thread has finished running (but it says "Record not available for unlocking"). Here, when I try with another fresh thread, the new thread and the remaining 2 threads are allowed to lock, book and unlock completely.
Any idea where could be my mistake?
Thanks
Nandini
[ August 16, 2003: Message edited by: Nandini Sriram ]

[Andrew Monkhouse]

Hi Nandini

If an attempt is made to unlock a record that has not been locked by this connection, then no action is to be taken.
My doubt is this - "no action to be taken" means the client's booking itself (lock - book - unlock) must be invalidated OR the booking allowed but ignore the call to unlock?

I believe that the book should not be allowed unless the connection owns a lock.
So attempting to unlock a record you have not locked should not affect any bookings because there cannot have been any bookings.
Regarding your recovery after client death scenario - it sounds like client B did not properly aquire the lock - perhaps when you woke client B up, they did not gain the lock, but just assumed that because they were now awake that they now owned the lock?
Regards, Andrew

[Nandini Sriram]

Hai Andrew,
Please take a look at my unreferenced code.

public void unreferenced () {
  Iterator it = lockedRecords.iterator();
     while (it.hasNext()){
       int record = ((Integer)it.next()).intValue();
       unlock (record);
     }
  System.out.println ("Unreferenced called");
}

Is this correct? The unreferenced should unlock all the records locked by the disconnected client, right?
Also Andrew, the client death scenario no longer seems to be a problem (at least that's what I think :roll: ).
What happens is this -
.... A locks #13 followed by B, C, D
.... A does not unlock ....
.... B C D are put to wait ....
.... unreferenced called and the record unlocked ...
.... B, C, D proceed to lock and unlock the record one by one ...
Is this correct???
How do I verify if the same client locks and unlocks a record? I'm printing the connection object (unique) while locking and unlocking to verify? Please suggest a better way.
Thanks
Nandini

[Andrew Monkhouse]

Hi Nandini

Please take a look at my unreferenced code.
...
Is this correct?

Have you tested it? Did it work? Or did you get a ConcurrentModificationException?
When you call the unlock() method, does it remove the record number from the lockedRecords collection?

.... A locks #13 followed by B, C, D
.... A does not unlock ....
.... B C D are put to wait ....
.... unreferenced called and the record unlocked ...
.... B, C, D proceed to lock and unlock the record one by one ...
Is this correct???

This looks like the right sequence.

How do I verify if the same client locks and unlocks a record? I'm printing the connection object (unique) while locking and unlocking to verify? Please suggest a better way.

Using the connection object to track ownership should be fine. You should be programatically comparing the connection object doing the unlock (or any modification methods) with the connection object who did the lock - as long as they match, you are fine.
Regards, Andrew

[Qusay Jaafar]

Andrew notes are valuable.
I wonder why unreferenced() method invoked while there isn't any crashing client according to what you said above.
A locked but not released the lock
B, C, D are waiting
till here, that's fine but A did not crash so I wonder why unreferenced() method invodked.
Unreferenced interface works only if there is no reference to a remote object.

[Andrew Monkhouse]

Hi Qusay
You are right, unreferenced will not be called until there is no longer a reference to the object.
I assumed that either A crashed or exited or invalidated it's reference to the remote object, causing unreferenced to be called.
Regards, Andrew

[Qusay Jaafar]

you are right andrew.
between the six minutes of my message and yours, I searched google for the word ("java"+"leaseValue").
I found this link which is:
When a RMI client obtains a reference for the remote object, by default, the reference is live for 10 minutes. If there is no client activity before the lease term expires, the reference is considered to be invalid, and the remote object may be subject to garbage collection, presuming there are no other clients holding a live reference to it.
However, the lease term can be easily changed, and is controlled by the system property java.rmi.dgc.leaseValue.
For example:
java -Djava.rmi.dgc.leaseValue=300000 MyRemoteImpl
resets the lease term to 5 minutes. The lease period is indicated in milliseconds.

[Nandini Sriram]

Hai Andrew and Qusay,
Andrew, CONGRATULATIONS!!! Just saw the post about your promotion. Very Happy to know that you will be the new bartender
Thanks to both of you for letting me know of all the probable places where I would have to re-check my code.
1. Let me tell you how I make the unreferenced call -
I comment out A's unlock, and then make it call lock. So the server calls unreferenced after 10 min.
...(Here ofcourse, at times, I do set a leaseValue of 30000 for the job to be over soon)...
Then I send 3 concurrent threads B C D to do the locking and unlocking. They seem to go fine!!!
I don't get exceptions here. Any other probability of something wrong somewhere?
Also, in my application, I have a 'close' button, which nulls the remote reference and calls System.gc(). So my unreferenced is called immediately after every connected client clicks 'close'. In case the client
doesn't click close but exits by closing the window directly, then unreferenced is called after 10 minutes.
Is this correct?

2. I have my booking method on my client side. In my DataClient, I have a booking method which performs the lock - modify - unlock in this fashion. So the main clientUI class just makes a call to this booking method.
Should the DataClient be devoid of references to the main UI, for eg., disabling a button in my UI class?
This sequence sees to it that a client acquires a lock before unlocking. But I also see to it that if lock is commented out,unlock spots that this object doesn't have a lock (by comparing the remote objects). Should I make my modify method also be aware of the calling object, so that no modification is done without a lock?
Thanks
Nandini

[Andrew Monkhouse]

Hi Nandini

Andrew, CONGRATULATIONS!!! Just saw the post about your promotion. Very Happy to know that you will be the new bartender

Thanks.

1. Let me tell you how I make the unreferenced call -
I comment out A's unlock, and then make it call lock. So the server calls unreferenced after 10 min.
...(Here ofcourse, at times, I do set a leaseValue of 30000 for the job to be over soon)...
Then I send 3 concurrent threads B C D to do the locking and unlocking. They seem to go fine!!!
I don't get exceptions here. Any other probability of something wrong somewhere?

Just out of curiosity, could you have your client lock two records before exiting? Then see if the clearing of locks in unreferenced works as expected.

Also, in my application, I have a 'close' button, which nulls the remote reference and calls System.gc(). So my unreferenced is called immediately after every connected client clicks 'close'. In case the client
doesn't click close but exits by closing the window directly, then unreferenced is called after 10 minutes.
Is this correct?

The first bit sounds good.
But what do you mean by "closing the window directly"? Do you mean "clicking on the little 'x' in the right hand corner of the OS title pane"? If so, why dont you catch that and have it run your exit routine as well?

Should I make my modify method also be aware of the calling object, so that no modification is done without a lock?

I did. I believe that the instructions hint that modifications should not be allowed without a lock being granted.
Regards, Andrew

[Nandini Sriram]

Hai Andrew,
I've finally found out what is wrong!!! I got the ConcurrentModificationException. I had commented out the
e.printStackTrace() :roll:
I get this exception thrown when I try to make 2 lockings by a single thread. I also found out it was because "if a thread modifies a collection directly while it is iterating over it with a fail-fast iterator, the iterator throws this exception."
What I have now done is this - instead of the set, I have an array. I go over the elements in the array and unlock them one by one.
1. With this I tried out the situation, where 1 client locked 2 records and did not unlock them, and another client put on wait. After the set time, the "unreferenced" was called after both the locked records were unlocked. The next client was able to lock and unlock after this. Is this correct, and do we have to clear all the locked records too,or enough just unlocking them?
2. Do we need to install a security manager?
3. I have a data object created in my factory and passed on to the connection object. So all connection objects get only one Data object. For my local access, I have a LocalDataImpl implementing RemoteIF, that creates a data object for local access. Is this acceptable?
4. I am doing a design documentation which tells about the design decisions, the various classes used on the server and client side, and a user guide like thing that describes and speaks of how to use the GUI etc.
Is this what is supposed to be as an "online" help HTML file?
Thanks,
Nandini
[ August 19, 2003: Message edited by: Nandini Sriram ]

[Andrew Monkhouse]

Hi Nandini

I got the ConcurrentModificationException

I am glad you stuck with it, and found that issue and then worked out why and fixed it - well done.

Is this correct, and do we have to clear all the locked records too,or enough just unlocking them?

I'm sorry, I don't understand what you are asking here.

2. Do we need to install a security manager?

No, you should not need a security manager.

3. I have a data object created in my factory and passed on to the connection object. So all connection objects get only one Data object. For my local access, I have a LocalDataImpl implementing RemoteIF, that creates a data object for local access. Is this acceptable?

Yes, this sounds fine.

... and a user guide like thing that describes and speaks of how to use the GUI etc.
Is this what is supposed to be as an "online" help HTML file?

The user guide can also be provided as online help (that is, you can get the help online within your application), but it is not essential. As long as you have the user guide available, that is all you need.
Regards, Andrew

[Qusay Jaafar]

my test class contains:

if ((Thread.currentThread().getName()).equals("--0--")) {
control.lock(7);
} else if ((Thread.currentThread().getName()).equals("--1--")) {
control.lock(5);
} else if ((Thread.currentThread().getName()).equals("--2--")) {
control.lock(7);
} else if ((Thread.currentThread().getName()).equals("--3--")) {
control.lock(5);
}

Interface Set documentation has not mentioned anything about ConcurrentModificationException.
Class HashSet documentation has mentioned it. also mentioned about how to solve this problem.
("If multiple threads access a set concurrently, and at least one of the threads modifies the set, it must be synchronized externally. This is typically accomplished by synchronizing on some object that naturally encapsulates the set").
In our case, lockedRecords who encapsulates the set. So we can code
unreferenced() method like this:

public void unreferenced() {
        synchronized (lockedRecords) {
            Set keySet = lockedRecords.keySet();
            Iterator it = keySet.iterator();
            while (it.hasNext()) {
                int record = ((Integer)it.next()).intValue();
                try {
                    unlock (record);
                } catch (RemoteException re) {
                    re.printStackTrace();
                }
            }
        }
}

Documentaion also mentiond
("If no such object exists, the set should be "wrapped" using the Collections.synchronizedSet method. This is best done at creation time, to prevent accidental unsynchronized access to the HashSet instance:
Set s = Collections.synchronizedSet(new HashSet(...));
")
So, I think we can code unreferenced() like this:

public void unreferenced() {
            Set keySet = mapLocks.keySet();
            Set s = Collections.synchronizedSet(new HashSet(keySet));
            Iterator it = keySet.iterator();
            while (it.hasNext()) {
                int record = ((Integer)it.next()).intValue();
                try {
                    unlock (record);
                } catch (RemoteException re) {
                    re.printStackTrace();
                }
            }
}

well, both ways, when I run it, ConcurrentModificationException were thrown up.
I am not sure if the statement ("This is best done at creation time") above was applied here correctly, or I did something wrong.
I know Nandini convert Set to array. I converted too according to his advice. It works fine, but I'd like to apply it as HashSet documentation mentioned.
any Idea?

[Andrew Monkhouse]

Hi Qusay
There are two separate issues with modifying the collections. The first one you noted ("If multiple threads access a set concurrently, and at least one of the threads modifies the set, it must be synchronized externally") is not what is causing the ConcurrentModificationException. This issue addresses the scenario where two threads are accessing the set at the same time, and one of them is modifying it. That is, if one thread was calling set.contains(value) while another thread was calling set.add(anotherValue) it is possible that the wrong result will be returned for the call to set.contains(). With maps it could be even worse - if you were calling the get() method, you might get the right value, a wrong value or no value at all!. So you have to synchronize all your accesses to the collection.
The issue regarding ConcurrentModificationException is described in the API documentation for HashSet as follows:

The iterators returned by this class's iterator method are fail-fast: if the set is modified at any time after the iterator is created, in any way except through the iterator's own remove method, the Iterator throws a ConcurrentModificationException. Thus, in the face of concurrent modification, the iterator fails quickly and cleanly, rather than risking arbitrary, non-deterministic behavior at an undetermined time in the future.

One way to get around this is to make an array as Nandini did, and walk over that array, unlocking the records.
The other way is as per the documentation: use the Iterator's remove method to remove the items from the set. Of course this means that you probably cannot use the standard unlock method of your remote class, since it is trying to directly modify the set. It depends on what else might be happening in your unlock method whether you want to refactor to get around that issue or just bypass the unlock method altogether.
Regards, Andrew

[Jim Yingst]

That is, if one thread was calling set.contains(value) while another thread was calling set.add(anotherValue) it is possible that the wrong result will be returned for the call to set.contains(). With maps it could be even worse - if you were calling the get() method, you might get the right value, a wrong value or no value at all!.
Or worse, either Map or Set might throw some RuntimeException like NullPointerException, ConcurrentAccessException, etc. If the underlying data structure is being changed by another thread, all sorts of weird things can happen. They don't document exactly what happens - they just document that you mustn't allow concurrent access if at least one of the threads is adding or removing elements.