Is my locking mechanism going to work?

[lambertlee Li]

I searched this forum and saw a lot of developers choose to have a lock manager.

I don't have a lock manager in my code. And I did not record down the client ID or client IP.

I just have a HashMap to record down the locked record no.
for example,

key=record No
value = System.getTimeInMiliSeconds()

Wherenever a client is going to delete/update, it should call methods in this order:

lock()
delete()/update()
unlock()

If client A is updating recNo=1, now client B also want to update recNo=1, it check that recNo=1 has been lock, then it has to wait.

//lock part
synchronized(locks){
while(locks.containsKey(key)){
try{
locks.wait();
}catch(InterruptedException e){
throw new Exception(e.getMessage());
}
}
lockCookie = System.currentTimeMillis();
locks.put(key, new Long(lockCookie));
}

//unlock part
synchronized(locks){
if(locks.containsKey(key)){
locks.remove(key);
locks.notifyAll();
}
}

All those work the same whether the client is runing in local mode or remote mode.

Any hidden issues in this design? Please comment. Thanks!

[Cindy Rogers]

Suggestion: check the documentation of the lockRecord method. It may contain the statement "If the specified record is already locked by a different client, the current thread gives up the CPU and consumes no CPU cycles until the record is unlocked."

With your current design, I believe the last part (consuming no CPU cycles until the record is unlocked) is not being satisfied.

An example:

1. Client A locks record 1.

2. Client B attempts to lock record 1, so is in wait state.

3. Client C locks record 2.

4. Client C unlocks record 2. Client B should remain in wait state (without doing any processing) because record 1 has not been unlocked.

[Andrew Monkhouse]

Do you have a statement along the lines of "Locks a record so that it can only be updated or deleted by this client"? If so, are you sure you are meeting that requirement?

Consider what happens in your code if:

Client A locks record 5

Client B attemts to lock record 5 (has to wait)

Client A deletes record 5

Client A unlocks record 5

After you have determined what will happen, re-read your instructions and see if that is reasonable or not.

Regards, Andrew

[lambertlee Li]

Hi Cindy,

Thanks so much your patience with my codes. But I don't understand.
In your example:

1. Client A locks record 1.

2. Client B attempts to lock record 1, so is in wait state.

3. Client C locks record 2.

4. Client C unlocks record 2.

Why Client B should remain in wait state? I thought after Client C unlocks record 2 will call notifyAll(), that will let client B know that the resource is available? Or I made stupid concept mistake? Help please.

[lambertlee Li]

Thanks Andrew,

In my instructions, I got this requirement on my locking:
Your server must be capable of handling multiple concurrent requests, and as part of this capability, must provide locking functionality as specified in the interface provided above. You may assume that at any moment, at most one program is accessing the database file; therefore your locking system only needs to be concerned with multiple concurrent clients of your server. Any attempt to lock a resource that is already locked should cause the current thread to give up the CPU, consuming no CPU cycles until the desired resource becomes available.

It seems no obvious requirements on "Locks a record so that it can only be updated or deleted by this client".

In your example:

Client A locks record 5
Client B attemts to lock record 5 (has to wait)
Client A deletes record 5
Client A unlocks record 5

Client B will find that record 5 has been deleted, and it will get RecordNotFoundException. But is this harm? What if I put those 3 methods inside a synchronized method, say

public synchronized void update/delete(){
lock()
delete()/update()
unlock()

}


But by this way, I will have double synchronized? isn't it? my lock() and unlock() already got synchronized block. I am so confused and frustrated.
Could anybody help me?

[Cindy Rogers]

It's a subtle difference: synchronizing on a single record versus all records. Currently your code is synchronizing on all records by using the locks object; that is, when any record is unlocked, all waiting clients are notified. The requirements state that the synchronization should be done on a record basis, so only those clients waiting for that specific record are notified.

Consider the stated requirement "If the specified record is already locked by a different client, the current thread gives up the CPU and consumes no CPU cycles until the record is unlocked".

If Client X is waiting for record 1, it should be notified only when record 1 is unlocked. It should not be notified if record 0 or record 2 is unlocked -- since that would consume CPU cycles.

[Robert Bar]

Hi,

It should not be notified if record 0 or record 2 is unlocked -- since that would consume CPU cycles.

- this assumption is too strict, isn't it? The "no cpu cycles" policy is taken too literaly, don't you think? IMO, "consume no CPU cycles" means: "aviod active waiting". I've decided to synchronize on locks map, and I feel good with this decision.

I'm waiting your comments about this issue.

[Cindy Rogers]

I don't know whether or not Sun requires the record level locking. I thought it was necessary based on the "consumes no CPU cycles" statement, but they may feel the file level lock is acceptable.

You may want to document your decision and rationale in the choices.txt document.

[Andrew Monkhouse]

Hi Kaymen,

In my instructions, I got this requirement on my locking:
Your server must be capable ...

It seems no obvious requirements on "Locks a record so that it can only be updated or deleted by this client".

I was thinking of a different section of the requirements. E.g. from my defintion of the lock() method:// Locks a record so that it can only be updated or deleted by this client.
// If the specified record is already locked, the current thread gives up
// the CPU and consumes no CPU cycles until the record is unlocked.
public void lock(int recNo) throws RecordNotFoundException;That very first line, or something similar to it, seems to be very common. You might want to double check whether you have something like it in yours.

Regards, Andrew

[lambertlee Li]

Hi Andrew,

Thanks very much. Yes, you are absolutely right, I have that line"Locks a record so that it can only be updated or deleted by this client" on the lock() method comments.

But I still don't understand what would happen will post issues based on my current design. Could you give me more detail? Or could you point out the defects on my current design?

[Andrew Monkhouse]

Hi kaymen,

I am just not seeing in your code where you are verifying that the client who locked a record is the only one who can then work with the record from that point onwards. Case in point: the unlock method only seems to check that the record is locked - it doesnt seem to care who locked it.

Regards, Andrew

[Robert Bar]

Hi,

IHO, signature and comments of methods may suggest required behaviour:

1.
long lock(recNo) throws RecordNotFoundException
void unlock(recNo, cookie) throws RecordNotFoundException, SecurityException
void delete(recNo, cookie) throws RecordNotFoundException, SecurityException

In this case client can unlock or delete records locked by himself. Any attempt to delete() or unlock() record without a cookie returned by lock() leads to SecurityException. Signature of methods enforces proper usage of interface. The risk of improper usage is lower than in second case (see below), but it's still present. In this case signatures of methods leave no doubts regaring the behaviour of Data class (comments are almost needless).

2.

void lock(recNo) throws RecordNotFoundException
void unlock(recNo) throws RecordNotFoundException
void delete(recNo) throws RecordNotFoundException

For me, this case is much more difficult than the first one. I'll describe one of the approaches which I have found:

If the specified record is already locked, the current thread always gives up (even record is locked by this client). Developer must ensure that he isn't going to lock the same record twice. Additionally there is no requirement about who can unlock a record (there is only requirement about deleting and updating record locked by this client). It is also possible to delete or update a record without using locking. I know that it's seems to be drastic, but it's up to the developer to ensure that interface is used properly! It's like using iterator pattern: normally, no one use next() before hasNext(). In this case developer should avoid lock() the same record twice, unlock() the record locked by other client, or delete() without lock() in the server environment, where multiple concurrent clients are present. On the other hand, its possible to implement standalone client without using lock()/unlock() and meet following requirement: "therefore your locking system only needs to be concerned with multiple concurrent clients of your server"

I'm waiting your comments !

regards, Robert

[lambertlee Li]

Hi, Guys,

Thanks for all your help. Maybe I did not state clearly in my first post.
I would like to describe clearly right now.

My design:

I don't have a lock manager in my code. And I did not record down the client ID or client IP.

I just have a static HashMap to record down the locked record no.
for example,

key=record No
value = System.getTimeInMiliSeconds()

Wherenever a client is going to delete/update, it should call methods in this order:

       lockCookie= lock(int recNo) ;
       delete(recNo, lockCookie)/update(recNo,data ,lockCookie)
unlock()

If client A is updating recNo=1, now client B also want to update recNo=1, it check that recNo=1 has been lock, then it has to wait.

//lock part
synchronized(locks){
while(locks.containsKey(key)){
try{
locks.wait();
}catch(InterruptedException e){
throw new Exception(e.getMessage());
}
}
lockCookie = System.currentTimeMillis();
locks.put(key, new Long(lockCookie));
}

//unlock part
synchronized(locks){
if(locks.containsKey(key)){
locks.remove(key);
locks.notifyAll();
}
}

All those work the same whether the client is runing in local mode or remote mode.

Any hidden issues in this design? Please comment. Thanks!

[lambertlee Li]

Hi, Guys,

Thanks for all your guys help. Maybe I did not state clearly in my first post. I would like to rewrite it.

My design:

I don't have a lock manager in my code. And I did not record down the client ID or client IP.

I just have a static HashMap to record down the locked record no.
for example,

key=record No
value = System.getTimeInMiliSeconds()

Wherenever a client is going to delete/update, it should call methods in this order:

       lockCookie= lock(int recNo) ;
       delete(recNo, lockCookie)/update(recNo,data ,lockCookie)
       unlock(recNo, lockCookie)

If client A is updating recNo=1, now client B also want to update recNo=1, it check that recNo=1 has been lock, then it has to wait.

//lock part

  synchronized(locks){
       while(locks.containsKey(key)){
          try{
                locks.wait();
       }catch(InterruptedException e){
                 throw new Exception(e.getMessage());
      }
   }
    lockCookie = System.currentTimeMillis();
    locks.put(key, new Long(lockCookie));
}

//unlock part

   synchronized(locks){
        if(locks.containsKey(key)){
            locks.remove(key);
            locks.notifyAll();
         }
    }

All those work the same whether the client is runing in local mode or remote mode.

I think this desing should satisfy the requirement of "Locks a record so that it can only be updated or deleted by this client". Comments please.

Also, I gathered information from other post mentioned this design have predefined client must do update action in this order lock->update->unlock. What if examiner will extract Data class and plug it into other tester or scenario which does not perform update in this order? I wonder how you guys solve this kind of problem. thanks!