1

My understanding of Java synchronized() blocks is that, if a thread already owns a lock on an object, it can enter a different block synchronized on the same object (re-entrant synchronization). Underneath, I believe that the JVM uses a reference count to increment/decrement the number of times a thread has acquired a lock, and that the lock is only released when the count is zero.

So my question is, if one encounters a piece of code that looks like this:

synchronized(this)
{
    if (condition == WAITING)
    {
        synchronized(this)
        {
            condition = COMPLETE;

            notify();

            try
            {
                wait();
            }
            catch(InterruptedException  e)
            {
            }
        }
    }
    else
        condition = READY;
}

what specifically happens when wait() is called? Does it merely decrement the count, or does it release the lock regardless of the count?

In the first case, it seems to me that it will produce a deadlock if lock re-entry had occurred, because it will still own the lock and would thus wait forever on another thread which is waiting on it.

In the second case, I can't really see what the point of the second synchronized block is at all.

The documentation for wait() says

"The current thread must own this object's monitor. The thread releases ownership of this monitor and waits until 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 thread then waits until it can re-obtain ownership of the monitor and resumes execution,"

so I think that the second case is the correct one, but I could be wrong. So am I missing something, or have I merely come across a redundant synchronized block that could just as easily be removed from the code?

  • If you see code like this (some of which doesn't compile) I suggest you delete and re-write it. Note: the first time you call notify, nothing will happen as no thread will be waiting. The second time, you won't call notify as the condition has changed, so your first thread will wait for ever. – Peter Lawrey Apr 16 '15 at 14:10
  • I (poorly) edited the code to simplify it as an example. The condition stuff and the uncompilable errors were me, not the code/issue itself. Also, condition is updated elsewhere, by another thread. The basic jist of the code is for a thread waiting on another thread to complete to be notified that the condition has changed. It, then, calls notify elsewhere when it has finished. – user2725525 Apr 16 '15 at 14:15
  • 1
    Why don't you rewrite this to use a Futures so that you know when a thread has completed? –  Apr 16 '15 at 14:26
  • a) Because I'm porting this to C++, not maintaining the current Java code, and b) Even so, 'complete' was a misnomer on my part. wait and notify are really dependent on internal state here, not any actual computational output. – user2725525 Apr 16 '15 at 15:25
  • Futures are not just for computations. Also C++ has futures and promise. –  Apr 17 '15 at 17:25

1 Answers1

6

There's nothing that necessitates the reacquiring of the lock after the if.

The wait() will also release the lock completely (otherwise it would be quite deadlock prone).

The only reason for the second synchronized that I can see, is that it previously used another object and someone modified it to use the same this by mistake.

Kayaman
  • 72,141
  • 5
  • 83
  • 121
  • That was my assumption as well. I'm porting a fairly large Java code base to C++, and trying to refactor it to avoid using std::recursive_mutex at all. Thanks for the information! – user2725525 Apr 16 '15 at 14:12
  • Just to confirm this scenario: when the thread encounters the outer synchronised block, it will acquire a lock on the object. It then encounters the second synchronised block which will trigger reentrancy. Notify is called but no threads can acquire a lock just yet as the current thread still has the object locked. Finally wait is called which cause the current thread to relinquish the lock it held. Do you concur? –  Apr 16 '15 at 14:21
  • Exactly. This also looks like a construct that could be handled much nicer with a class from `java.util.concurrent`, but I suppose the code is old and the author may not have been familiar with them. – Kayaman Apr 16 '15 at 14:27
  • Isn't your statement: *There's nothing that warrants the reacquiring of the lock after the if.* misleading? The reentrancy will cause the lock-counter to increment. In other words, the second synchronised block **will** warrant reacquiring the lock. –  Apr 16 '15 at 14:32
  • Fixed wording. English isn't my native language, so maybe I shouldn't be using fancy language constructs :) Incrementing the lock counter only makes sense in recursive situations so that returning from a recursive synchronized context doesn't give up the lock too early. However there's nothing like that here (since `wait()` doesn't care how many times it's synchronized on the same object). – Kayaman Apr 17 '15 at 05:12
  • @Kayaman, it's still not correct. When the current thread encounters the second synchronised block it will make the current thread reentrant (as you agreed with my earlier analysis). So after the `if`, there **is** something which will necessitate reacquiring the lock. –  Apr 17 '15 at 06:21
  • I'm not sure I understand you. Locks are reentrant, not threads. The second synchronized blog doesn't "trigger reentrancy". The reentrancy is an implicit property. Then `notify()` wakes up another thread and starts `wait()`ing, allowing the other thread to run. There is no difference between this code and if the inner synchronized were to be removed. – Kayaman Apr 17 '15 at 06:44
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/75488/discussion-between-i-k-and-kayaman). –  Apr 17 '15 at 09:58