8153107: Unbalanced recursive locking

Andrey Petushkov andrey.petushkov at gmail.com
Wed Jun 13 12:40:38 UTC 2018


Dear Hotspot developers,

It looks like the cause of the https://bugs.openjdk.java.net/browse/JDK-8153107 <https://bugs.openjdk.java.net/browse/JDK-8153107> problem is that ObjectSynchronizer::quick_enter implementation leaves the BasicLock marked as recursive (DHW==0) in case that the lock is inflated and already taken. As such the routines which first check the recursive stack locking (e.g. fast_exit) consider this lock as non-inflated recursive and hence do not decrement _recursions field of ObjectMonitor, leaving the monitor locked forever. In contrary the slow path does work the right way: ObjectSynchronizer::slow_enter first marks stack lock as non-locked and non-recursive (DHW==3) and then delegates to ObjectMonitor::enter to increment _recursions.
So from the source code prospective this bug looks like platform-independent, contrary to what’s was found under https://bugs.openjdk.java.net/browse/JDK-8131715 <https://bugs.openjdk.java.net/browse/JDK-8131715>. Unfortunately I cannot prove it with an example. The only case it fails for me is runtime/CreateMirror/ArraysNewInstanceBug.java hotspot jtreg test on aarch32 openjdk port. The following changes fix the problem:

diff --git a/src/hotspot/share/runtime/sharedRuntime.cpp b/src/hotspot/share/runtime/sharedRuntime.cpp
--- a/src/hotspot/share/runtime/sharedRuntime.cpp
+++ b/src/hotspot/share/runtime/sharedRuntime.cpp
@@ -1985,9 +1985,7 @@
 JRT_BLOCK_ENTRY(void, SharedRuntime::complete_monitor_locking_C(oopDesc* _obj, BasicLock* lock, JavaThread* thread))
   // Disable ObjectSynchronizer::quick_enter() in default config
   // on AARCH64 and ARM until JDK-8153107 is resolved.
-  if (ARM_ONLY((SyncFlags & 256) != 0 &&)
-      AARCH64_ONLY((SyncFlags & 256) != 0 &&)
-      !SafepointSynchronize::is_synchronizing()) {
+  if (!SafepointSynchronize::is_synchronizing()) {
     // Only try quick_enter() if we're not trying to reach a safepoint
     // so that the calling thread reaches the safepoint more quickly.
     if (ObjectSynchronizer::quick_enter(_obj, thread, lock)) return;
diff --git a/src/hotspot/share/runtime/synchronizer.cpp b/src/hotspot/share/runtime/synchronizer.cpp
--- a/src/hotspot/share/runtime/synchronizer.cpp
+++ b/src/hotspot/share/runtime/synchronizer.cpp
@@ -220,11 +220,6 @@
     // Case: light contention possibly amenable to TLE
     // Case: TLE inimical operations such as nested/recursive synchronization
 
-    if (owner == Self) {
-      m->_recursions++;
-      return true;
-    }
-
     // This Java Monitor is inflated so obj's header will never be
     // displaced to this thread's BasicLock. Make the displaced header
     // non-NULL so this BasicLock is not seen as recursive nor as
@@ -237,6 +232,11 @@
     // and last are the inflated Java Monitor (ObjectMonitor) checks.
     lock->set_displaced_header(markOopDesc::unused_mark());
 
+    if (owner == Self) {
+      m->_recursions++;
+      return true;
+    }
+
     if (owner == NULL && Atomic::replace_if_null(Self, &(m->_owner))) {
       assert(m->_recursions == 0, "invariant");
       assert(m->_owner == Self, "invariant");

Regards,
Andrey


More information about the hotspot-runtime-dev mailing list