• Post Reply Bookmark Topic Watch Topic
  • New Topic
programming forums Java Mobile Certification Databases Caching Books Engineering Micro Controllers OS Languages Paradigms IDEs Build Tools Frameworks Application Servers Open Source This Site Careers Other Pie Elite all forums
this forum made possible by our volunteer staff, including ...
Marshals:
  • Campbell Ritchie
  • Jeanne Boyarsky
  • Ron McLeod
  • Paul Clapham
  • Liutauras Vilda
Sheriffs:
  • paul wheaton
  • Rob Spoor
  • Devaka Cooray
Saloon Keepers:
  • Stephan van Hulst
  • Tim Holloway
  • Carey Brown
  • Frits Walraven
  • Tim Moores
Bartenders:
  • Mikalai Zaikin

Comments on lock/unlock please

 
Greenhorn
Posts: 26
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I assume that the calls will follow the sequence lock/modify/unlock, I haven't tracked the client info. I will justify this in the designchoices.txt. The following is what my lock, unlock method look like.
Regarding lock db request, I declared one static variable called "DB_LOCK_REQUEST" for letting other coming threads wait until my unlock method to set "DB_LOCK_REQUEST" to false. Any comments are welcome.

[Andrew: Put code between [code] and [/code] UBB Tags]
[ November 28, 2003: Message edited by: Andrew Monkhouse ]
 
Jianhua Ren
Greenhorn
Posts: 26
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Sorry, I forgot to use UBBCODE standard in my previous post, so the code format is very ugly. I decided to post again with nice format of my code.
 
author and jackaroo
Posts: 12200
280
Mac IntelliJ IDE Firefox Browser Oracle C++ Java
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Hi Jianhua,
Looks like you posted again at the same time as I edited your original code. For future information, if you want to you can always go back and edit your own posts. Above each individual post is the icon. If you click on that, you will be taken to a page where you can edit your post.
In your general comments you mention that you do not track the client info. But in your javadoc comments for the unlock() method, you state that the call to unlock() is ignored if the caller does not have a current lock on the requested method. How are you handling this?
The if statement in the following code appears redundant. Why do you have both an if statement and a while statement that are doing the same check?

I believe you are doing FBNS. Is that correct?
If so, you have changed the lock() method signature to include a RecordNotFoundException - have you specified why you added this in your documentation?
Regards, Andrew
 
Jianhua Ren
Greenhorn
Posts: 26
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Thanks Andrew for your reply! I appreciate your time!
--------------------------------------------------------------------------------
if (DB_LOCK_REQUEST || lockedRecords.contains(recordNo)) { while (DB_LOCK_REQUEST || lockedRecords.contains(recordNo)) {
--------------------------------------------------------------------------------
<hr></blockquote>
I have to validate if the record is still exist in the database or not outside the while loop and inside the if statement. Because after the waiting thread(called T1) got mutex lock, we have to make sure the record is still in database, since another thread might delete the record from the database while this thread (T1) is waiting. And I have validate() method in the top place in the method, so I don't what the thread which doesn't need to wait to validate record two times, I just use if() to avoid this.


believe you are doing FBNS. Is that correct?
If so, you have changed the lock() method signature to include a RecordNotFoundException - have you specified why you added this in your documentation?


Yes, I am working old FBNS. I could recreate RecordNotFoundException which extends IOException, then I don't need to declare method throwing RecordNotFoundException. I really don't want to change the signatures actually, but tracking client id is hard to implement without changing signature.


[ November 28, 2003: Message edited by: Jianhua Ren ]
[ November 28, 2003: Message edited by: Jianhua Ren ]
 
Jianhua Ren
Greenhorn
Posts: 26
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Andrew,

The if statement in the following code appears redundant. Why do you have both an if statement and a while statement that are doing the same check?


After I reconsider my code, I made a modification on my lock method, it looks like the following, no duplicate checking there now:

1. Regarding the validation of record. I think we have to put the validation in the two places, one place is in the very begining of the lock method, if the record is invalid, no need to do futher work at all; another place is just after waiting thread gets mutex lock, because maybe the record in which the waiting thread is intrested has been deleted already by another thread while the waiting thread was in the wait status.
2. Regarding the lock whole database. I used a static variable DB_LOCK_REQUEST to monitor the database lock request and actual database lock action, no new lock is allowed once the database lock request is monitored(threads with new lock request have to sit there waiting), after all existing locks finish the lock action and unlock the locks, the database got locked; DB_LOCK_REQUEST will be disabled in the unlock method, then all the waiting threads will be notified. If a new database lock request is monitored while there is existing data base lock request there, the thread with new database lock request has to wait. I just added the new piece of code above to handle this case.
Any comments? I am missing sth here?
Thanks,
Jianhua
 
Ranch Hand
Posts: 4982
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I have a through here:

How can you make sure that, the record still valid after you checked the validity, and before you lock the record?
Since you have not synchronized the data file, thus, thread 1 checks the record is valid, and continue to perform, however, thread 2 deletes the record at // POINT 1.
Nick.
 
Jianhua Ren
Greenhorn
Posts: 26
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Hi Nick,
Thanks for checking this! I think you're right, I have to take off my first validation method and move my second validation method outside the while loop, just before lockRecords.add() method. By doing so, assuming thread A is holding the lock and trying to delete the record 1, thread B which wants to modify record 1 comes, thread B will wait until thread A finishes its work, releasees the lock, thread B gets the lock, at this moment no any thread is working on record 1, so we can put validation of record 1 method here. How do you think about this, Nick?
Thanks,
Jianhua
 
Nicholas Cheung
Ranch Hand
Posts: 4982
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
I suddenly have a through, now, our code is:

However, if we synchronized locker in lock(), even if other thread wanna release their lock, they cannot do so becos it is hold by the current thread. Am I correct?
Please advice.
Thanks.
Nick.
 
Jianhua Ren
Greenhorn
Posts: 26
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator


However, if we synchronized locker in lock(), even if other thread wanna release their lock, they cannot do so becos it is hold by the current thread. Am I correct?


The case you mentioned would not happen.
The lock we think is that the record number is contained in the collection of lockedRecords, we say Thread 1 holds the lock of record 1, that means "1" is contained in the collection, so as long as Thread 1 still holds the lock of record 1("1" is not removed from collection), other threads cannot hold that lock of record 1, since the while loop will make them to wait.
If Thread 1 wants to release it's lock on record 1, it must mean that collection of lockedRecords contains "1" at that moment, since lock/operation/unlock logic has to be followed. So when other threads which want to do sth on record 1 come, they have to call their lock method first, they will find that "1" is in the collection already, then they have to sit there waiting, only after Thread 1 finishes it's work on record, removes "1" from collection of lockedRecords, the waiting threads for record 1 would be notified, then go to compete for the lock of record 1.
Make sense?
Jianhua
 
Nicholas Cheung
Ranch Hand
Posts: 4982
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
My concern is:
Assume there are 2 threads, T1 and T2, and both of them want to operate with record R1.
If T1 starts first, it will:
lock(R1);
// T2 starts at this point
perform operation on it
unlock(R1);
If T2 starts right after T1 locks R1;
It tries to lock R1, but fail becos R1 is in the lock, it waits (Up to here, still no problem).
However, consider the code:

Although T2 is waiting, it enters the "critical section" of locker, and waiting inside the synchronized block!!!
i.e. if T1 does not release its lock, T2 waits until T1 release.
However, think about T1. When it wants to release the lock R1, it will:

But T1 will wait at the POINT X, becos, by refering to T2, T2 enters the critical section, so T1 cannot enter into it.
The case will then become: T2 waits for T1 to release the lock, T1 waits for T2 to give up the critical section. Thus, the case becomes deadlocked.
So, could we only synchronize the locker in the lock() while we do not need to synchronize the locker in unlock()?
Many thanks for your advice.
Nick.
 
Jianhua Ren
Greenhorn
Posts: 26
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator


But T1 will wait at the POINT X, becos, by refering to T2, T2 enters the critical section, so T1 cannot enter into it.


I finally knows what you're thinking. But one thing you should notice is that wait method does release the lock, it's different with sleep method and yield method(sleep and yiele still holds lock while it sleeps or yields).
So T1 will not wait at the POINT X, it will get into the synchronized block of unlock method, since before T2 waits, it releases the lock of collection of locker first due to calling of wait method.
You can check the Max's book on page 81/82, he explains the wait method and gives the example.
Hope this helps!
Jianhua
 
Bartender
Posts: 1872
  • Mark post as helpful
  • send pies
    Number of slices to send:
    Optional 'thank-you' note:
  • Quote
  • Report post to moderator
Hi Jianhua,
Here are a few quick comments on the last version of lock() you posted :

  • Are you sure your lock() method may throw an IOException ?
  • Your first call to validateRecord() is useless IMO. Think of the fact that if the record is invalid, nobody will have a lock on it, in such a way that you won't wait for it.
  • Your second call to validateRecord() should go out of the while loop (just before lockedRecords.add()) : your goal is not to validate the record each time you get a chance to grab the lock, but when you actually can do it.
  • recordNo declaration could go in the else block.
  • DB_LOCK_REQUEST looks like a constant. dbLockRequest is more appropriate.
  • I see a big issue with the way you get a database lock : let's say that T1 owns a db lock and T2 and T3 are waiting for such a db lock. When T1 will release its db lock, T2 *AND* T3 will be granted the db lock. You should replace the if(dbLockRequest) by a while(dbLockRequest).


  • Best,
    Phil.
     
    Andrew Monkhouse
    author and jackaroo
    Posts: 12200
    280
    Mac IntelliJ IDE Firefox Browser Oracle C++ Java
    • Mark post as helpful
    • send pies
      Number of slices to send:
      Optional 'thank-you' note:
    • Quote
    • Report post to moderator
    Hi Jianhua,
    Just some extra nitpicking on the code in addition to Phil's excellent comments (by the way Phil - Jianhua is working on FBNS which does have an IOException in the lock method signature).

    Normally anything in all capitals is considered a constant. This code would confuse me, because I would expect DB_LOCK_REQUEST to be a constant, in which case there would be no point in checking what it's value is. (I know the next line you assign a value to DB_LOCK_REQUEST so I can work out that it is not a constant, but it is still not good practice.

    This is one place that I would expect to find a constant. It usually makes the code a bit more readable if you use some constant such as LOCK_ALL_DATABASE instead of the hard coded number -1.
    Also, you have a requirement that only the client that locks a record may unlock it. How are you handling that? (I suspect that you are handling it the same way I handled it (and therefore I think you are right ) - but I just want to check that you have not forgotten about that requirement).
    Regards, Andrew
     
    Jianhua Ren
    Greenhorn
    Posts: 26
    • Mark post as helpful
    • send pies
      Number of slices to send:
      Optional 'thank-you' note:
    • Quote
    • Report post to moderator
    Philippe, thank you very much for your comments.

    Are you sure your lock() method may throw an IOException ?


    Yes, lock() in my assignment does throw IOException, I am working on FBNS.

    Your first call to validateRecord() is useless IMO. Think of the fact that if the record is invalid, nobody will have a lock on it, in such a way that you won't wait for it.
    Your second call to validateRecord() should go out of the while loop (just before lockedRecords.add()) : your goal is not to validate the record each time you get a chance to grab the lock, but when you actually can do it.


    You are right. Actually I already took off the first call to validateRecord(), and moved the second call outside the while loop already. You may just overlooked that message I posted in the middle part of this thread. But, still want to thank you for pointing that out.

    I see a big issue with the way you get a database lock : let's say that T1 owns a db lock and T2 and T3 are waiting for such a db lock. When T1 will release its db lock, T2 *AND* T3 will be granted the db lock. You should replace the if(dbLockRequest) by a while(dbLockRequest).


    This is my fault. Yes, I have to change it to the while loop. What a comment it is!! Thanks again, Philippe!!
    Andrew,
    Thank you very much for your comment!!
    I will take care of the variable name issue.

    Also, you have a requirement that only the client that locks a record may unlock it. How are you handling that? (I suspect that you are handling it the same way I handled it (and therefore I think you are right ) - but I just want to check that you have not forgotten about that requirement).


    Yes, I am not forgetting that. I think I will handle it the way you did. Keeping track the info of clients and records which they lock outside the Data class, only when the client which locks the record comes, the unlock call will be forwarded to Data class's unlock without changing the signature of unlock() of Data class.
    Thanks,
    Jianhua
     
    And then we all jump out and yell "surprise! we got you this tiny ad!"
    a bit of art, as a gift, that will fit in a stocking
    https://gardener-gift.com
    reply
      Bookmark Topic Watch Topic
    • New Topic