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 aarch32-port-dev
mailing list