SCJD failed, help needed with locking!

[Jack North]

My SCJD assignment is URLyBird 1.1.1. I got my results couple a days ago and
everything else was fine, except locking, as you can see below:

General Considerations: 100/100 
Documentation:           70/70 
O-O Design:              30/30 
GUI                      31/40 
Locking                   0/80  
Data store               40/40 
Network server           40/40

The assessor gave me a hint what was wrong: "Major point loss for record-locking mechanism, which is not according to spec. Your lock() method calls addReservation(),
which locks and then unlocks."

The specification of the lock-method was:

/**
 * Locks a record so that it can only be updated or deleted by this
 * client
 * 
 * Returned value is a cookie that must be used when the record is
 * unlocked, updated, or deleted. If the specified record is already
 * locked by different client, the current thread gives up the CPU and
 * consumes no CPU cycles until the record is unlocked
 */

The contents of my lock-method was:

  public long lock(int recNo) throws RecordNotFoundException {
Room room = database.getRoom(recNo);
if (room == null) {
    throw new RecordNotFoundException();
}
return reservationsManager.addReservation(recNo);
  }

And reservationsManager:

public class ReservationsManager {
    private static Lock lock = new ReentrantLock();
    /** recNo -> lockCookie map * */
    private static Map<Integer, Long> reservations = new HashMap<Integer, Long>();
    /** lock cookie * */
    private static long lockCookie = 0;
 
    ....
    
    public long addReservation(int recNo) {
      lock.lock();
      
  try {
    if (reservations.containsKey(recNo)) {
    throw new SecurityException("Record already reserved");
    }
    lockCookie++;
    reservations.put(recNo, lockCookie);
       } finally {
    lock.unlock();
  }
  return lockCookie;
    }
}

Is the problem that my addReservation method does NOT "give up the CPU and
consume no CPU cycles until the record is unlocked"? If so,

1) Is this enough to make my locking points go all the way to 0?
2) why does the assessor says that "Your lock() method calls addReservation(),
which locks and then unlocks." Is this a hint that my addReservation should
do more than just lock an unlock (missing waiting the release of the record) or
something else?

[John Stone]

1) I'm afraid so.
2) Hint could be better, but I guess it suggests, that your thread should wait, rather than throwing 'SecurityException("Record already reserved");'

[Jack North]

Well I changed my addReservation and removeReservation methods:

class ReservationsManager {
 
 private static Lock lock = new ReentrantLock();
 private static Condition lockReleased = lock.newCondition();
 
 public long addReservation(int recNo) {
lock.lock();
try {
    while(reservations.containsKey(recNo)) {
    lockReleased.await();
    }
  
    lockCookie++;
    reservations.put(recNo, lockCookie);
}
catch(InterruptedException e) {
    throw new SecurityException("interrupted problem");
}
finally {
    lock.unlock();
}
 
 }
 

 public void removeReservation(int recNo, long lockCookie)
    throws RecordNotFoundException, SecurityException {
 
lock.lock();
try {
    checkReservation(recNo, lockCookie); // throws securityexception, if recNo
                                         // and lockCookie does not match
    reservations.remove(recNo);
    lockReleased.signal();               // signal to waiting threads that
                                         // a record has been unlocked
} finally {
    lock.unlock();
}
 
 }
}

I was wondering if there was a problem with this scenario

- Client 1 locks record no. 1
- Client 2 tries to lock record no. 1. Because record is locked,
client will wait until it is unlocked.
- Client 3 locks record no. 2. Makes some update and unlocks the
record.

When client 3 makes these actions, removeReservation method is called.
removeReservation signals (lockReleased.signal()) that a lock is released.

After this, client 2 doesn't wait anymore, immediately client 2 checks in
the while loop if the released lock was record no. 1. Since it is not, waiting continues:

    while(reservations.containsKey(recNo)) {
    lockReleased.await();
    }

The question is, is this according to specification, since the specification
says that "the current thread gives up the CPU and consumes no CPU cycles until the
record is unlocked". In this case, the current thread has to check if the released
record was the record that the thread is trying to lock. And that way the thread
really consumes CPU cycles?

[John Stone]

Regarding "signal" vs. "signalAll", you can check my old thread:
Denny's DVD ReservationsManager - signal()

"And that way the thread really consumes CPU cycles?"
Well, it does a little, but compared to active waiting it should not be a problem. You would need to implement per record waiting to avoid waking up threads, which are waiting upon different records.

[Jack North]

Regarding "signal" vs. "signalAll", you can check my old thread:

Did I understand correctly, I should use lockReleased.signalAll(); instead of lockReleased.signal()?

Well, it does a little, but compared to active waiting it should not be a problem. You would need to implement per record waiting to avoid waking up threads, which are waiting upon different records.

Well I'm wondering if I would return my assignment as above, would I get 0 points because my lock-method is not according to the spec still (it consumes CPU..)

[John Stone]

What do you think?
Can you see any danger, if let's say:

50 threads are waiting for record 1
1 thread is waiting for record 2
somebody unlocks record 2 and calls signal()

-------

I think, that Andrew mentioned in his book, that per-record locking is not needed for exam. Try to search in Certification results, if you can find any ranchers who passed without per record locking. I think you will find some.

[Chris Be]

Jack,

I think you have an ambiguity issue here:

       } finally {
    lock.unlock();
  }

I understand that, on a higher level, the key locking mechanism options in the SCJD are:

1. Achieve locking with some use of a java.util.concurrent.locks.Lock implementation in combination with a container to keep track of the record lock objects.
2. Achive locking with an own implementation using synchronization, some container for keeping track of which record is locked, and wait()/sleep()/notify()/notifyAll()

It appears that you use a hybrid of using synchronization via the ReentrantLock object (Approach 1) and a synchronized reservations map (Approach 2) to achieve locking. Whereby the reservations map is actually maintaining the locked status, and the lock object is merely used as a substitute for the synchronized keyword, not as an actual lock object as designed by Sun.

This is a somewhat counterintuitive, especially for junior programmers - and apparently testers. With the given design, I can follow the tester's train of thought when he would expect a ReentrantLock to be locked when leaving the method (thinking along the lines of approach 1).

You may have added an entry to the reservations map, but do not actually hold a lock, since you may have wanted to keep track of your lock status in the reservations map (Approach 2).

Options:

Use the lockable object pattern and store ReentrantLock objects in your lock map instead, then lock the respective lock object for a given record. This way the tester gets what he was looking for: a locked object upon leaving the lock() method. Since it is a ReentrantLock, you can then re-lock, when deleting, updating etc. to ensure concurrency is maintained throughout your transactions.

[Jack North]

I'm pretty confused what I should do. There are two choices:

1) Implement system where all waiting threads are notified
2) Implement lock per record system

The advantage of the no. 2 is that it consumes less cpu than no. 1. Disadvantage
for me is that it is more complicated to implement and that might lead to a system
that has more errors. No 1. is easier to implement but it not might be what the
assessor wants.

Andrew says in his book

"We have presented this section on having multiple notification objects as it does
appear to meet a requirement listed in some of the sun instructions. However,
at the time of this writing Sun does not appear to penalize those candidates who
ignore this requirement"

There were no mention about multiple notification's on my instructions. This was
the only instruction about locking:

**
 * Locks a record so that it can only be updated or deleted by this
 * client
 * 
 * Returned value is a cookie that must be used when the record is
 * unlocked, updated, or deleted. If the specified record is already
 * locked by different client, the current thread gives up the CPU and
 * consumes no CPU cycles until the record is unlocked
 */

Also there seems to be cases where people have passed SCJD test with locking
system that is not per record.

Since I only need 9 points to pass I think if I choose no. 1 that would
increase my locking-points atleast the needed 9 points?

What do you think?

[Jack North]

Originally posted by Chris Be:

This is a somewhat counterintuitive, especially for junior programmers - and apparently testers. With the given design, I can follow the tester's train of thought when he would expect a ReentrantLock to be locked when leaving the method (thinking along the lines of approach 1).

I don't think that one would get 0 points if the solution was just counterintuitive? In my original solution there was no waiting at all..

[uzma ali]

Hi

I think Chris has a point as according to my understanding

   
finally {    lock.unlock();  } 

should not be in the reservationManager() method , the reason being
if you are unlocking it already so when you do the update and delete then obviously the record is not locked as you already did it before leaving the method.

What do you say. That what I developed the understanding after getting stuck for quite a bit. Please suggest.

UZma

[archana kher]

The issue is not about code. I think you misunderstood the assignment.
There are 2 concepts involved here 'cookie assignment' and 'locking a record'.

Holding cookie by a client means, client does a reservation to manipulate the record. There can be more than one client wanting to manipulate a record. Every client wishing to manipulate the data will need to acquire cookie. As a result there can be more than one cookies (clients) associated with a record.

Holding 'lock' means holding exclusive right to change the record.

Its madatory to acquire 'cookie' before 'lock' in order to perform record manipulation.

Hope this helps!!!