Why isn't Object.notify() a synchronized method?
Hi all, in the Javadoc of notify(), notifyAll() and wait(...) I read, that this methods should only be used with synchronisation on it's instance. So I'm wondering, why they don't have the synchronized modifier out of the box in Object class. Also I think, the following note should be moved from wait(long,int) to wait(long): /The current thread must own this object's monitor. The thread releases ownership of this monitor and waits until either of the following two conditions has occurred:// / * /Another thread notifies threads waiting on this object's monitor to wake up either through a call to the notify method or the notifyAll method./ * /The timeout period, specified by timeout milliseconds plus nanos nanoseconds arguments, has elapsed. / Cheers, Ulf
What would synchronized on that method help with? Typically you do something while holding the lock and then notify while still holding the lock. sent from my phone On May 28, 2015 12:09 PM, "Ulf Zibis" <Ulf.Zibis@cosoco.de> wrote:
Hi all,
in the Javadoc of notify(), notifyAll() and wait(...) I read, that this methods should only be used with synchronisation on it's instance. So I'm wondering, why they don't have the synchronized modifier out of the box in Object class.
Also I think, the following note should be moved from wait(long,int) to wait(long): /The current thread must own this object's monitor. The thread releases ownership of this monitor and waits until either of the following two conditions has occurred:// /
* /Another thread notifies threads waiting on this object's monitor to wake up either through a call to the notify method or the notifyAll method./ * /The timeout period, specified by timeout milliseconds plus nanos nanoseconds arguments, has elapsed. /
Cheers,
Ulf
Since most of the time you have to hold the lock anyway for other reasons, I think this would generally be an unwelcome change since I expect the cost of recursive lock acquisition is nonzero. On 05/28/2015 11:08 AM, Ulf Zibis wrote:
Hi all,
in the Javadoc of notify(), notifyAll() and wait(...) I read, that this methods should only be used with synchronisation on it's instance. So I'm wondering, why they don't have the synchronized modifier out of the box in Object class.
Also I think, the following note should be moved from wait(long,int) to wait(long): /The current thread must own this object's monitor. The thread releases ownership of this monitor and waits until either of the following two conditions has occurred:// /
* /Another thread notifies threads waiting on this object's monitor to wake up either through a call to the notify method or the notifyAll method./ * /The timeout period, specified by timeout milliseconds plus nanos nanoseconds arguments, has elapsed. /
Cheers,
Ulf
-- - DML
Thanks for your hint David. That's the only reason I could imagine too. Can somebody tell something about the cost for recursive lock acquisition in comparison to the whole call, couldn't it be eliminated by Hotspot? As I recently fell into the trap of forgetting the synchronized block around a single notifyAll(), I believe, the current situation is just errorprone. Any comments about the Javadoc issue? -Ulf Am 28.05.2015 um 18:27 schrieb David M. Lloyd:
Since most of the time you have to hold the lock anyway for other reasons, I think this would generally be an unwelcome change since I expect the cost of recursive lock acquisition is nonzero.
On 05/28/2015 11:08 AM, Ulf Zibis wrote:
Hi all,
in the Javadoc of notify(), notifyAll() and wait(...) I read, that this methods should only be used with synchronisation on it's instance. So I'm wondering, why they don't have the synchronized modifier out of the box in Object class.
Also I think, the following note should be moved from wait(long,int) to wait(long): /The current thread must own this object's monitor. The thread releases ownership of this monitor and waits until either of the following two conditions has occurred:// /
* /Another thread notifies threads waiting on this object's monitor to wake up either through a call to the notify method or the notifyAll method./ * /The timeout period, specified by timeout milliseconds plus nanos nanoseconds arguments, has elapsed. /
Cheers,
Ulf
On 30/05/2015 2:48 AM, Ulf Zibis wrote:
Thanks for your hint David. That's the only reason I could imagine too. Can somebody tell something about the cost for recursive lock acquisition in comparison to the whole call, couldn't it be eliminated by Hotspot?
There are a number of fast paths related to recursive locking. Not sure if inlining allows lock elision that something for the compiler folk.
As I recently fell into the trap of forgetting the synchronized block around a single notifyAll(), I believe, the current situation is just errorprone.
How is it errorprone? You forgot to acquire the lock and you got an IllegalMonitorStateException when you did notifyAll. That's pointing out your error.
Any comments about the Javadoc issue?
I did comment - I don't see any issue. David H.
-Ulf
Am 28.05.2015 um 18:27 schrieb David M. Lloyd:
Since most of the time you have to hold the lock anyway for other reasons, I think this would generally be an unwelcome change since I expect the cost of recursive lock acquisition is nonzero.
On 05/28/2015 11:08 AM, Ulf Zibis wrote:
Hi all,
in the Javadoc of notify(), notifyAll() and wait(...) I read, that this methods should only be used with synchronisation on it's instance. So I'm wondering, why they don't have the synchronized modifier out of the box in Object class.
Also I think, the following note should be moved from wait(long,int) to wait(long): /The current thread must own this object's monitor. The thread releases ownership of this monitor and waits until either of the following two conditions has occurred:// /
* /Another thread notifies threads waiting on this object's monitor to wake up either through a call to the notify method or the notifyAll method./ * /The timeout period, specified by timeout milliseconds plus nanos nanoseconds arguments, has elapsed. /
Cheers,
Ulf
On Sun, May 31, 2015 at 02:31:25PM +1000, David Holmes wrote:
As I recently fell into the trap of forgetting the synchronized block around a single notifyAll(), I believe, the current situation is just errorprone.
How is it errorprone? You forgot to acquire the lock and you got an IllegalMonitorStateException when you did notifyAll. That's pointing out your error.
The reason for not making wait/notify synchronized is *not* that "it would be unnecessary because you typically already hold the lock anyway". The reason is (as David Holms pointed out earlier) that it would be *meaningless* to make them synchronized. When you say it's errorprone it sounds like you first had notifyAll(); and then "fixed" the IllegalMonitorStateException by doing synchronized (this) { notifyAll(); } (and then wrote your original email asking why notifyAll didn't do this "on the inside"). If this is the case you have not understood the intention of using synchronized here. In a nutshell wait and notify is all about thread communication, and for that to work correctly you need to synchronize your execution. Let me try to explain why wrapping *just* notifyAll() in a synchronized block (or relying on making notifyAll synchronized) is broken: Suppose you have a producer / consumer thing going on. In produce() you have something like enqueue(value); synchronized (this) { notifyAll(); // because consumer may be waiting for a value } and in consume() you have something like synchronized (this) { while (noValueIsAvailable()) wait(); } value = retrieve(); Suppose now that ProducerThread enters produce(), enqueues a value and before reaching notifyAll, ConsumerThread comes along, enters consume(), consumes the value, processes the value, calls consume *again*, sees that noValueIsAvailable and calls wait. ProducerThread now continues it's execution and calls notifyAll(). ConsumerThread is woken up, but at this point no value is available despite there was a call to notify! (In the best case, this doesn't crash the program, in worst case, ProducerThread assumed that the value should be processed after notifyAll, in which case you may run into a deadlock. If there were more variables involved you could also have memory visibility issues involved and accidentally break class invariants by doing this.) I've written an answer to a similar question here: "Why must wait() always be in synchronized block" http://stackoverflow.com/a/2779674/276052 best regards, Andreas
The performance issue here is mostly a red herring. The reason notify() is not synchronized has much more to do with correctness; when you "forget" to wrap your notify call with lock acquisition, it is almost always a bug. (The rest of this explanation is probably clearer if you've read JCiP Ch14.) A thread calls notify/notifyAll if it has effected a state change that could cause a waiting thread to become unblocked. Blocking is generally associated with a state predicate ("queue is nonempty", "light is green"), which refers to some state, and that state needs to be guarded by the lock associated with the condition queue. If you've modified the state that underlies the predicate (i.e., you put something in the queue, or switched the light to green), you needed the lock anyway. (Because spurious wakeup is allowed, even the trivial cases like a one-shot "release all threads when timer expires" are better implemented with a boolean state predicate (or better, CountDownLatch) than simply calling notifyAll.) So, while its not out of the question that code that wants to call notify without locking isn't wrong, it probably is wrong, and having the library "conveniently" acquire the lock for you is just brushing the mistake under the rug. On 5/29/2015 12:48 PM, Ulf Zibis wrote:
Thanks for your hint David. That's the only reason I could imagine too. Can somebody tell something about the cost for recursive lock acquisition in comparison to the whole call, couldn't it be eliminated by Hotspot?
As I recently fell into the trap of forgetting the synchronized block around a single notifyAll(), I believe, the current situation is just errorprone.
Any comments about the Javadoc issue?
-Ulf
Am 28.05.2015 um 18:27 schrieb David M. Lloyd:
Since most of the time you have to hold the lock anyway for other reasons, I think this would generally be an unwelcome change since I expect the cost of recursive lock acquisition is nonzero.
On 05/28/2015 11:08 AM, Ulf Zibis wrote:
Hi all,
in the Javadoc of notify(), notifyAll() and wait(...) I read, that this methods should only be used with synchronisation on it's instance. So I'm wondering, why they don't have the synchronized modifier out of the box in Object class.
Also I think, the following note should be moved from wait(long,int) to wait(long): /The current thread must own this object's monitor. The thread releases ownership of this monitor and waits until either of the following two conditions has occurred:// /
* /Another thread notifies threads waiting on this object's monitor to wake up either through a call to the notify method or the notifyAll method./ * /The timeout period, specified by timeout milliseconds plus nanos nanoseconds arguments, has elapsed. /
Cheers,
Ulf
On 05/28/2015 07:08 PM, Ulf Zibis wrote:
Hi all,
in the Javadoc of notify(), notifyAll() and wait(...) I read, that this methods should only be used with synchronisation on it's instance. So I'm wondering, why they don't have the synchronized modifier out of the box in Object class.
Well, at least "synchronized" wait() would be very odd, since it actually relinquishes the monitor. This will pose an interesting paradox: if wait() is claimed to be synchronized, but notify() requires acquiring the same monitor, how would any object be notified, ever? But really, I'd think this serves the purpose of "waking up" with all the appropriate locking state preserved. E.g. making sure only one thread owns the object monitor, and it "reenters" back right where it had left at wait() in the synchronized section. Thanks, -Aleksey
On 29/05/2015 2:08 AM, Ulf Zibis wrote:
Hi all,
in the Javadoc of notify(), notifyAll() and wait(...) I read, that this methods should only be used with synchronisation on it's instance. So I'm wondering, why they don't have the synchronized modifier out of the box in Object class.
Because, as others have said, the way you use a monitor is to acquire the monitor lock, then inspect/modify mutable object state, then do a wait/notify/notifyAll as appropriate, and finally release the monitor. Making the methods themselves synchronized would be useless from a synchronization correctness perspective and would prevent coding errors from resulting in IllegalMonitorStateException.
Also I think, the following note should be moved from wait(long,int) to wait(long): /The current thread must own this object's monitor. The thread releases ownership of this monitor and waits until either of the following two conditions has occurred:// /
* /Another thread notifies threads waiting on this object's monitor to wake up either through a call to the notify method or the notifyAll method./ * /The timeout period, specified by timeout milliseconds plus nanos nanoseconds arguments, has elapsed. /
Why would you move it when wait(long) already has the more detailed: * <p> * The current thread must own this object's monitor. * <p> * This method causes the current thread (call it <var>T</var>) to * place itself in the wait set for this object and then to relinquish * any and all synchronization claims on this object. Thread <var>T</var> * becomes disabled for thread scheduling purposes and lies dormant * until one of four things happens: ... David -----
Cheers,
Ulf
participants (7)
-
Aleksey Shipilev
-
Andreas Lundblad
-
Brian Goetz
-
David Holmes
-
David M. Lloyd
-
Ulf Zibis
-
Vitaly Davidovich