[aarch64-port-dev ] 8153107: Unbalanced recursive locking

[Andrew Haley]

On 06/13/2018 01:40 PM, Andrey Petushkov wrote:

> 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:

Ouch.  The code is due to 8167501, which was a bug in the Oracle ARM64
and ARM32 ports.  This bug was never seen in the AARCH64 open port,
so the workaround shouldn't be enabled for it.

8167501: ARMv7 Linux C2 compiler crashes running jtreg harness on MP systems
Reviewed-by: dcubed
diff -r afd6ae4fec81 -r 71d7ced6c439 hotspot/src/share/vm/runtime/sharedRuntime.cpp
--- a/hotspot/src/share/vm/runtime/sharedRuntime.cpp	Thu Nov 03 11:53:20 2016 +0530
+++ b/hotspot/src/share/vm/runtime/sharedRuntime.cpp	Thu Nov 03 10:44:17 2016 -0400
@@ -1983,8 +1983,10 @@
 // Handles the uncommon case in locking, i.e., contention or an inflated lock.
 JRT_BLOCK_ENTRY(void, SharedRuntime::complete_monitor_locking_C(oopDesc* _obj, BasicLock* lock, JavaThread* thread))
   // Disable ObjectSynchronizer::quick_enter() in default config
-  // on AARCH64 until JDK-8153107 is resolved.
-  if (AARCH64_ONLY((SyncFlags & 256) != 0 &&) !SafepointSynchronize::is_synchronizing()) {
+  // on AARCH64 and ARM until JDK-8153107 is resolved.
+  if (ARM_ONLY((SyncFlags & 256) != 0 &&)
+      AARCH64_ONLY((SyncFlags & 256) != 0 &&)
+      !SafepointSynchronize::is_synchronizing()) {
     // Only try quick_enter() if we're not trying to reach a safepoint

Why did you move the code which handles recursive locking counts?  Do
you believe that it's incorrect where it is, and if so, why?

-- 
Andrew Haley
Java Platform Lead Engineer
Red Hat UK Ltd. <https://www.redhat.com>
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671