aspose file tools*
The moose likes Developer Certification (SCJD/OCMJD) and the fly likes Doubt about using notify() Big Moose Saloon
  Search | Java FAQ | Recent Topics | Flagged Topics | Hot Topics | Zero Replies
Register / Login
JavaRanch » Java Forums » Certification » Developer Certification (SCJD/OCMJD)
Bookmark "Doubt about using notify()" Watch "Doubt about using notify()" New topic
Author

Doubt about using notify()

Guillaume Drouet
Ranch Hand

Joined: Apr 16, 2012
Posts: 36
Hi every one,

I have finished my URLyBird (v1.1.3) application and I am currently writing my choices file. I am a bit anxious about what I read in this topic because I have chosen to use notify() against notifyAll().

I don't have any deadlock issue (I execute the Roberto's DB test class successfully) but I want to expose my design choices in order to have your opinion.

Each record is represented by an object (Record class) in a memory cache. When a record is locked (is cookie is not null), I call the wait() method on this record object. When the thread that owns the lock calls the unlock method, notify() is called on the record object. One of the waiting threads is awaken, do it job, and finally calls the unlock(). unlock() invokes internally the notify() method on the record object to awake another thread and so on. Multiple records can be locked at the same time. This is how I implement my locking mechanisms using notify().

lock() and unlock() are called in the same method of my data access object class (example : book() calls lock(), update() and unlock() in a finally block). I don't implement any timeout mechanism because I consider that unlocking is guarantee by the finally block that calls unlock().

In this kind of design, I think notifying all threads that are waiting for a lock on a record makes no sense because only one thread will get the lock and others will back in a waiting state. In my choices.txt, I plan to explain that I will get better performance using notify(). Another discussion point is the meaning of the event thrown when calling notify(). Can I justify that notifying a thread that is waiting for a record object ALWAYS means that the record has been unlocked ?

Do you think that I can get full credit with notify() and this design ?

Thanks.
Roel De Nijs
Bartender

Joined: Jul 19, 2004
Posts: 5408
    
  13

Guillaume Drouet wrote:Do you think that I can get full credit with notify() and this design ?

If you decide to synchronize each record-object (instead of the more global this-object or a data member) notify will be the only correct choice, using notifyAll would make no sense at all and show that you don't understand the difference between them (and that might result in some points being deducted). And you are correct: it's a more performant solution than synchronization on this-object and using notifyAll (because less overhead and less threads being notified). But a more performant solution will not receive automatically better grade than a less performant one (as per instructions of the assignment).

Guillaume Drouet wrote:Another discussion point is the meaning of the event thrown when calling notify(). Can I justify that notifying a thread that is waiting for a record object ALWAYS means that the record has been unlocked ?

In my solution notifyAll (because I synchronized on the this-object) is only called from the unlock-method when the lock on a record is released. Just like wait() is called if and only if the record is successfully locked.


Hope it helps!


SCJA, SCJP (1.4 | 5.0 | 6.0), SCJD
http://www.javaroe.be/
Guillaume Drouet
Ranch Hand

Joined: Apr 16, 2012
Posts: 36
Hi !

Thanks for your reply. I can continue to write my choices without modifying my code, it is reassuring !

The performance is not the point where I think I will get more credit but in that part, mentioning that notify() is more efficient than notifyAll() would show that I understand the difference between them and illustrate my design decision.

I use wait() and notify() in lock() and unlock() too so I will explain that notifying always means that a record has been unlocked. Actually, I call notify() in delete() too. In that case, a thread that is waiting for locking the record will raise a RNFE after being notified when calling lock().
Roel De Nijs
Bartender

Joined: Jul 19, 2004
Posts: 5408
    
  13

Guillaume Drouet wrote:Actually, I call notify() in delete() too.

Why? It's the responsability of unlock-method to release lock and notify (all) waiting threads, not the delete-method.
Guillaume Drouet
Ranch Hand

Joined: Apr 16, 2012
Posts: 36
It is a good question. I do not remember if there was a technical reason beind this or if it was just a point of view.

Maybe I thought that it makes no sense to unlock a record that does not exist anymore. Do you think I can justify this point of view ?
Roel De Nijs
Bartender

Joined: Jul 19, 2004
Posts: 5408
    
  13

Guillaume Drouet wrote:Do you think I can justify this point of view ?

In my opinion the delete-method has just 1 purpose: deleting a record. The unlock-method has also 1 purpose: unlocking the record (notifying waiting threads is part of the unlocking-process). If you let the delete-method also notify waiting threads, your design/implementation loses a bit cohesion (1 purpose for each method). That's why I'm totally against it, it makes no sense at all. You call lock before delete-method, so you would expect you have to call unlock too...
Guillaume Drouet
Ranch Hand

Joined: Apr 16, 2012
Posts: 36
Well after replaying my tests, I remember that I call notify() in the delete method in order to solve a deadlock issue but my design still requires to call unlock() after the object has been deleted (I reuse deleted entries so the object is kept in the memory cache).
Roel De Nijs
Bartender

Joined: Jul 19, 2004
Posts: 5408
    
  13

Guillaume Drouet wrote:I call notify() in the delete method in order to solve a deadlock issue

Huh? Sounds to me like you have a flaw in your design. Why do you need a waiting thread to be woken up while the record is still locked? Makes no sense at all. Only the unlock-method should call notify (or notifyAll).
Guillaume Drouet
Ranch Hand

Joined: Apr 16, 2012
Posts: 36
In fact, I think I have missed something because when I delete entries I have deadlock issues. I think it was a bad idea to call notify on the records in the memory cache because it is always done on a local variable. My lock method do this :
1. call getRecord() (internal method that throws RNFE)
2. synchronize on the record
3. if the cookie of the record is not null, then I call wait() and then call recursively lock()
4. else generate cookie and set it to the record

I think I have to forget the recursively call of lock() after the thread wakes up and the use of wait() and notify() on records. Do you think it is a better idea to use the concurrent API and add a Lock member to each record object in order to replace the wait() / notify() mechanism ?
Roel De Nijs
Bartender

Joined: Jul 19, 2004
Posts: 5408
    
  13

Your approach will only work if the record-objects are instance variables, with local variables it makes no sense at all (local variables are always thread-safe). You could use seperate lock objects per record, but these also should be stored as instance variables (in a map or something similar). So possible memory issues is definitely a drawback of this approach.
Guillaume Drouet
Ranch Hand

Joined: Apr 16, 2012
Posts: 36
Actually, I do something like this :



The local variable is created from an instance stored in a map declared as class member.

I'm a bit confused about issues generated by this mechanism...

Using lock object in a separate map will have the same result, no ?
Roel De Nijs
Bartender

Joined: Jul 19, 2004
Posts: 5408
    
  13

If your getRecord-method is creating a new instance of Record by each call it won't work at all! And it's even worse: you'll definitely fail, because you don't have any concurrency Because each thread will have it's own instance of a Record object. If getRecord-method is just a convenience method to retrieve a Record instance from a map (which is an instance member), then multiple threads will have to compete for lock on same object.
Guillaume Drouet
Ranch Hand

Joined: Apr 16, 2012
Posts: 36
Roel De Nijs wrote:If getRecord-method is just a convenience method to retrieve a Record instance from a map (which is an instance member), then multiple threads will have to compete for lock on same object.


It's the case ! getRecord always calls get() on the memory map that stores the records. When I call wait() and notify() on a record, it always refers to the same instance (which is in the memory map).

It is very sad that I keep this deadlock issue. Moreover, the deadlock occurs very rarely (0 to 2/3 times for 400 threads in a test !). I solve it by calling notify() in my delete method (problem is related to deleted records) but as you say it is not a good design...
Guillaume Drouet
Ranch Hand

Joined: Apr 16, 2012
Posts: 36
I think I have found a good solution to my problem.

Deadlock occurs when a thread acquires the lock on a record that has just been deleted. In this case, I throw a RNFE by checking if the record is deleted (a method isDeleted() provided by the record object gives this information). Before throwing a RNFE, I call notifyAll() in order to wake up all the threads waiting for locking this record. At this moment, they also come to throw a RNFE.

In this particular case, I call notifyAll() in my lock method. Do you confirm it is a better solution than calling notify() in my delete() method ?
Roel De Nijs
Bartender

Joined: Jul 19, 2004
Posts: 5408
    
  13

You found the cause of your problem, which is a good thing

This issue demonstrates you should use notifyAll instead of notify (in the unlock-method). That's the one and only good solution! Your solution would also work, but although it's a bit more performant, it's a lot more complicated and not that intuitive. And don't forget that a clean and easy design is preferred above a complex but more performant one.
Guillaume Drouet
Ranch Hand

Joined: Apr 16, 2012
Posts: 36
Well I actually continue to use notify() in the unlock() method, it is only when the record is deleted where I call notifyAll() in my lock() method because I know that all records will not back in a waiting state. notify() and notifyAll() are used in my design for appropriated operations.

In fact, it is more complicated :
- I combine notify() and notifyAll()
- n lock objects (for n records in the data file) are used instead of only one

In plan to justify those choices by :
- showing that I make an appropriate use of notify() and notifyAll() according to the context. This will show that I understand the advantages of each method and their differences
- explaining that it is more efficient but more progressive too. In fact : currently, my DAO (which uses the data class instance) unlock a record very quickly after locking it. In that case, performance are not really better for users than a solution where only one record could be locked at a given time. Now imagine that tomorrow you have a treatment that takes times (2/3 seconds) between the lock and the unlock of a record (for instance calling a web service that retrieves some data to use to update a record). If 3 users perform an operation at the same time, one of them will wait 6 to 9 seconds before the treatment ends successfully ! Very bad performances would be considered has an issue in that case. I think it is an interesting point when you read that "URLyBird wants to move into Internet-based marketing, and hopes to be able to accept bookings direct from customers over the web". Migrating on the Internet could implies new features like my example. Moreover, the section that mention that "A clear design, such as will be readily understood by junior programmers, will be preferred to a complex one, even if the complex one is a little more efficient" is contained in a section called "Clarity and Maintainability". To guarantee an easy maintenance for junior programmers, the best way is to be sure that will not have to modify the locking mechanisms because of performance issues.
Roel De Nijs
Bartender

Joined: Jul 19, 2004
Posts: 5408
    
  13

Guillaume Drouet wrote:Now imagine that tomorrow you have a treatment that takes times (2/3 seconds) between the lock and the unlock of a record (for instance calling a web service that retrieves some data to use to update a record). If 3 users perform an operation at the same time, one of them will wait 6 to 9 seconds before the treatment ends successfully ! Very bad performances would be considered has an issue in that case.

A web service taking 2-3 seconds to finish its work is a very slow web service which definitely can use some performance tweaking But I don't see why your solution (with a lock-object for each record and just a notify in unlock-method) would be more performant in this scenario than a solution with just 1 lock-object and a notifyAll in unlock-method). Could you elaborate a bit more on this one?
Guillaume Drouet
Ranch Hand

Joined: Apr 16, 2012
Posts: 36
Imagine you have 3 records : #1, #2 and #3

Each record has a user who wants to lock it : there is a thread A for locking #1, thread B for locking #2 and thread C for locking #3

In a solution with only one lock object and notifyAll(), you can only have a situation like this :
1. Thread A locks #1
2. Thread B waits
3. Thread C waits
4. Thread A unlocks #1
5. Thread B wakes up
6. Thread C wakes up
7. Thread B locks #2
8. Thread C waits
9. Thread B unlocks #2
10 Thread C wakes up
11. Thread C locks #3
12. Thread C unlocks #3

In my solution :
1. Thread A locks #1
2. Thread B locks #2
3. Thread C locks #3
4. Thread A unlocks #1
5. Thread B unlocks #2
6. Thread C unlocks #3

A fourth thread will wait if it tries to lock #1, #2 or #3 but it could not have to wait if it locks another record.

A webservice that takes 3 seconds could be considered too slow but it could be normal that a treatment takes 3 seconds between the call of lock() and unlock(). Considering this, in the first solution, waiting time will increase for thread B and more for thread C :
1. Thread A locks #1 - second : 0
2. Thread B waits - second : 0
3. Thread C waits - second : 0
4. Thread A unlocks #1 - second : 3
5. Thread B wakes up - second : 3
6. Thread C wakes up - second : 3
7. Thread B locks #2- second : 3
8. Thread C waits - second : 3
9. Thread B unlocks #2 - second : 6
10 Thread C wakes up - second : 6
11. Thread C locks #3 - second : 6
12. Thread C unlocks #3 - second : 9

When thread C unlocks the record #3, it is done 9 seconds after the call of lock() because it has to wait that thread A and thread B do there work and release the one and only lock of the application.

My solution is not perfect because if three threads try to lock the same record, it will have the same result. But it occurs more rarely that users work on the same record than on different records at the same time.
Roel De Nijs
Bartender

Joined: Jul 19, 2004
Posts: 5408
    
  13

In a solution with only one lock object and notifyAll(), you can only have a situation like this :
1. Thread A locks #1
2. Thread B waits
3. Thread C waits
4. Thread A unlocks #1
5. Thread B wakes up
6. Thread C wakes up
7. Thread B locks #2
8. Thread C waits
9. Thread B unlocks #2
10 Thread C wakes up
11. Thread C locks #3
12. Thread C unlocks #3

That's not true!

With the given scenario and in a solution with only 1 lock object (and notifyAll), you'll have his sitation:
1. Thread A locks #1
2. Thread B locks #2
3. Thread C locks #3
4. Thread A unlocks #1
5. Thread B unlocks #2
6. Thread C unlocks #3

That's exactly the same as your solution, because a thread has to wait only if the record it wants to lock is already locked by another thread. So you don't have to wait at all for 6-9 seconds (because 3 threads are interested in different records). Only if 3 threads lock the same record, but that's for both solutions.
Guillaume Drouet
Ranch Hand

Joined: Apr 16, 2012
Posts: 36
Ho I see what I have missed. Thanks for your correction. In fact if a record does not have a cookie set, then the thread will not call wait() on the lock object.

I hope my solution will be validated and don't loose too much points !
 
I agree. Here's the link: http://aspose.com/file-tools
 
subject: Doubt about using notify()