review v4

Author: xmas92

src/hotspot/share/runtime/lightweightSynchronizer.cpp

@@ -1029,6 +1029,9 @@ ObjectMonitor* LightweightSynchronizer::inflate_and_enter(oop object, BasicLock*
   }
 
   if (monitor->try_enter(locking_thread)) {
+    if (monitor->has_successor(current)) {
+      monitor->clear_successor();
+    }
     return monitor;
   }
 
@@ -1144,6 +1147,7 @@ ObjectMonitor* LightweightSynchronizer::inflate_and_enter(oop object, BasicLock*
 
   if (current == locking_thread) {
     monitor->enter_with_contention_mark(locking_thread, contention_mark);
+    assert(!monitor->has_successor(current), "what?");
   } else {
     monitor->enter_for_with_contention_mark(locking_thread, contention_mark);
   }

src/hotspot/share/runtime/objectMonitor.cpp

@@ -461,6 +461,9 @@ bool ObjectMonitor::spin_enter(JavaThread* current) {
 
   // Check for recursion.
   if (try_enter(current)) {
+    if (has_successor(current)) {
+      clear_successor();
+    }
     return true;
   }
 
@@ -475,6 +478,9 @@ bool ObjectMonitor::spin_enter(JavaThread* current) {
   // Note that if we acquire the monitor from an initial spin
   // we forgo posting JVMTI events and firing DTRACE probes.
   if (try_spin(current)) {
+    if (has_successor(current)) {
+      clear_successor();
+    }
     assert(has_owner(current), "must be current: owner=" INT64_FORMAT, owner_raw());
     assert(_recursions == 0, "must be 0: recursions=%zd", _recursions);
     assert_mark_word_consistency();
@@ -492,7 +498,6 @@ bool ObjectMonitor::enter(JavaThread* current) {
   }
 
   assert(!has_owner(current), "invariant");
-  assert(!has_successor(current), "invariant");
   assert(!SafepointSynchronize::is_at_safepoint(), "invariant");
   assert(current->thread_state() != _thread_blocked, "invariant");
 
@@ -551,6 +556,12 @@ void ObjectMonitor::enter_with_contention_mark(JavaThread* current, ObjectMonito
     notify_contended_enter(current);
     result = Continuation::try_preempt(current, ce->cont_oop(current));
     if (result == freeze_ok) {
+      if (has_successor(current)) {
+        clear_successor();
+        // Invariant: after setting succ=null a contending thread
+        // must recheck-retry _owner before parking. (Done in vthread_monitor_enter)
+        OrderAccess::fence();
+      }
       bool acquired = vthread_monitor_enter(current);
       if (acquired) {
         // We actually acquired the monitor while trying to add the vthread to the
@@ -927,7 +938,9 @@ void ObjectMonitor::enter_internal(JavaThread* current) {
 
   // Try the lock - TATAS
   if (try_lock(current) == TryLockResult::Success) {
-    assert(!has_successor(current), "invariant");
+    if (has_successor(current)) {
+      clear_successor();
+    }
     assert(has_owner(current), "invariant");
     return;
   }
@@ -941,7 +954,7 @@ void ObjectMonitor::enter_internal(JavaThread* current) {
   // to the owner.  This has subtle but beneficial affinity
   // effects.
 
-  if (try_spin(current)) {
+  if (final_try_spin(current)) {
     assert(has_owner(current), "invariant");
     assert(!has_successor(current), "invariant");
     return;
@@ -1106,7 +1119,7 @@ void ObjectMonitor::reenter_internal(JavaThread* current, ObjectWaiter* currentN
 
     // If that fails, spin again.  Note that spin count may be zero so the above TryLock
     // is necessary.
-    if (try_spin(current)) {
+    if (final_try_spin(current)) {
         break;
     }
 
@@ -2393,12 +2406,22 @@ bool ObjectMonitor::try_spin(JavaThread* current) {
     _SpinDuration = adjust_down(_SpinDuration);
   }
 
+  return false;
+}
+
+bool ObjectMonitor::final_try_spin(JavaThread* current) {
+  if (try_spin(current)) {
+    if (has_successor(current)) {
+      // We are locked, clear the successor.
+      clear_successor();
+    }
+    return true;
+  }
+
   if (has_successor(current)) {
     clear_successor();
     // Invariant: after setting succ=null a contending thread
-    // must recheck-retry _owner before parking.  This usually happens
-    // in the normal usage of try_spin(), but it's safest
-    // to make try_spin() as foolproof as possible.
+    // must recheck-retry _owner before parking.
     OrderAccess::fence();
     if (try_lock(current) == TryLockResult::Success) {
       return true;

src/hotspot/share/runtime/objectMonitor.hpp

@@ -456,6 +456,7 @@ class ObjectMonitor : public CHeapObj<mtObjectMonitor> {
   TryLockResult  try_lock(JavaThread* current);
 
   bool      try_spin(JavaThread* current);
+  bool      final_try_spin(JavaThread* current);
   bool      short_fixed_spin(JavaThread* current, int spin_count, bool adapt);
   void      exit_epilog(JavaThread* current, ObjectWaiter* Wakee);