src/share/vm/runtime/objectMonitor.cpp

*** 380,389 ****
--- 380,395 ----
      JavaThreadBlockedOnMonitorEnterState jtbmes(jt, this);
  
      DTRACE_MONITOR_PROBE(contended__enter, this, object(), jt);
      if (JvmtiExport::should_post_monitor_contended_enter()) {
        JvmtiExport::post_monitor_contended_enter(jt, this);
+ 
+       // The current thread does not yet own the monitor and does not
+       // yet appear on any queues that would get it made the successor.
+       // This means that the JVMTI_EVENT_MONITOR_CONTENDED_ENTER event
+       // handler cannot accidentally consume an unpark() meant for the
+       // ParkEvent associated with this ObjectMonitor.
      }
  
      OSThreadContendState osts(Self->osthread());
      ThreadBlockInVM tbivm(jt);
  
*** 437,446 ****
--- 443,458 ----
    // spinning we could increment JVMStat counters, etc.
  
    DTRACE_MONITOR_PROBE(contended__entered, this, object(), jt);
    if (JvmtiExport::should_post_monitor_contended_entered()) {
      JvmtiExport::post_monitor_contended_entered(jt, this);
+ 
+     // The current thread already owns the monitor and is not going to
+     // call park() for the remainder of the monitor enter protocol. So
+     // it doesn't matter if the JVMTI_EVENT_MONITOR_CONTENDED_ENTERED
+     // event handler consumed an unpark() issued by the thread that
+     // just exited the monitor.
    }
  
    if (event.should_commit()) {
      event.set_klass(((oop)this->object())->klass());
      event.set_previousOwner((TYPE_JAVALANGTHREAD)_previous_owner_tid);
*** 1454,1463 ****
--- 1466,1483 ----
       // post monitor waited event.  Note that this is past-tense, we are done waiting.
       if (JvmtiExport::should_post_monitor_waited()) {
          // Note: 'false' parameter is passed here because the
          // wait was not timed out due to thread interrupt.
          JvmtiExport::post_monitor_waited(jt, this, false);
+ 
+         // In this short circuit of the monitor wait protocol, the
+         // current thread never drops ownership of the monitor and
+         // never gets added to the wait queue so the current thread
+         // cannot be made the successor. This means that the
+         // JVMTI_EVENT_MONITOR_WAITED event handler cannot accidentally
+         // consume an unpark() meant for the ParkEvent associated with
+         // this ObjectMonitor.
       }
       if (event.should_commit()) {
         post_monitor_wait_event(&event, 0, millis, false);
       }
       TEVENT (Wait - Throw IEX) ;
*** 1497,1521 ****
     _waiters++;                  // increment the number of waiters
     _recursions = 0;             // set the recursion level to be 1
     exit (true, Self) ;                    // exit the monitor
     guarantee (_owner != Self, "invariant") ;
  
-    // As soon as the ObjectMonitor's ownership is dropped in the exit()
-    // call above, another thread can enter() the ObjectMonitor, do the
-    // notify(), and exit() the ObjectMonitor. If the other thread's
-    // exit() call chooses this thread as the successor and the unpark()
-    // call happens to occur while this thread is posting a
-    // MONITOR_CONTENDED_EXIT event, then we run the risk of the event
-    // handler using RawMonitors and consuming the unpark().
-    //
-    // To avoid the problem, we re-post the event. This does no harm
-    // even if the original unpark() was not consumed because we are the
-    // chosen successor for this monitor.
-    if (node._notified != 0 && _succ == Self) {
-       node._event->unpark();
-    }
- 
     // The thread is on the WaitSet list - now park() it.
     // On MP systems it's conceivable that a brief spin before we park
     // could be profitable.
     //
     // TODO-FIXME: change the following logic to a loop of the form
--- 1517,1526 ----
*** 1593,1603 ****
--- 1598,1627 ----
       // (Don't cache naked oops over safepoints, of course).
  
       // post monitor waited event. Note that this is past-tense, we are done waiting.
       if (JvmtiExport::should_post_monitor_waited()) {
         JvmtiExport::post_monitor_waited(jt, this, ret == OS_TIMEOUT);
+ 
+        if (node._notified != 0 && _succ == Self) {
+          // In this part of the monitor wait-notify-reenter protocol it
+          // is possible (and normal) for another thread to do a fastpath
+          // monitor enter-exit while this thread is still trying to get
+          // to the reenter portion of the protocol.
+          //
+          // The ObjectMonitor was notified and the current thread is
+          // the successor which also means that an unpark() has already
+          // been done. The JVMTI_EVENT_MONITOR_WAITED event handler can
+          // consume the unpark() that was done when the successor was
+          // set because the same ParkEvent is shared between Java
+          // monitors and JVM/TI RawMonitors (for now).
+          //
+          // We redo the unpark() to ensure forward progress, i.e., we
+          // don't want all pending threads hanging (parked) with none
+          // entering the unlocked monitor.
+          node._event->unpark();
         }
+      }
  
       if (event.should_commit()) {
         post_monitor_wait_event(&event, node._notifier_tid, millis, ret == OS_TIMEOUT);
       }