JDK-8268347 : C2: nested locks optimization may create unbalanced monitor enter/exit code
  • Type: Bug
  • Component: hotspot
  • Sub-Component: compiler
  • Affected Version: 8,9,11,16,17
  • Priority: P2
  • Status: Closed
  • Resolution: Fixed
  • Submitted: 2021-06-07
  • Updated: 2023-09-25
  • Resolved: 2021-06-14
The Version table provides details related to the release that this issue/RFE will be addressed.

Unresolved : Release in which this issue/RFE will be addressed.
Resolved: Release in which this issue/RFE has been resolved.
Fixed : Release in which this issue/RFE has been fixed. The release containing this fix may be available for download as an Early Access Release or a General Availability Release.

To download the current JDK release, click here.
JDK 11 JDK 17 JDK 18
11.0.13Fixed 17 b27Fixed 18Fixed
Related Reports
Relates :  
Relates :  
Relates :  
Relates :  
Description
There are several restrictions on when nested locks should be optimized. One of them is that NLO requires one BoxLock node per one lock/unlock region.
Unfortunately loop unswitching optimization can create such a case because loop cloning does not clone BoxLock nodes which are outside loops (they are pinned to the Root node). Note, Lock/Unlock nodes are a subclass of Call node - all loop optimizations are skipped, except unswitching.

NLO happens during Macro nodes elimination. But before that Lock/Unlock coarsening optimization happens during IGVN as early as right after parsing - even before Escape analysis.

Now back to the failing case:

while (a) {
    synchronized(obj) { // ORn (outer lock region)
          if (loop_invar_check) {
              synchronized(obj) { // NR1 } // (nested lock region 1)
          } else {
             // no synchronization
          }
          synchronized(obj) { // NR2 } // (nested lock region 2)
    }
}

After loop unswitching:

 if (loop_invar_check) {
    while (a) {
        synchronized(obj) { // ORn (outer lock region)
             synchronized(obj) { // NR1 } // (nested lock region 1)
             synchronized(obj) { // NR2 } // (nested lock region 2)
        }
    }
} else {
    while (a) {
        synchronized(obj) { // CORn (Clone of outer lock region)
             synchronized(obj) { // CNR2 } // (Clone of nested lock region 2)
        }
    }
}

After loop unswitching for the code in the first branch Lock/Unlock coarsening optimization marked Unlock node from NR1 and Lock from NR2 as "Coarsened".

When NLO (Nested Lock Optimization) started, it overwrote the state for Unlock node from NR1 as "Nested" because it passed all conditions: it belonged to a simple region with one BoxLock node and was inside ORn region.
But NLO did not overwrite the state of Lock node from NR2 because this region shares BoxLock node with its clone CNR2 in second branch of unswitched loop. As a result this Lock node stayed "Coarsened". Later, when locks are eliminated, Lock/Unlock nodes in NR1 are eliminated as "Nested". And Lock node in NR2 is eliminated as  "Coarsened" leaving behind an un-matching Unlock node from NR2.


Comments
The fix added the check and assert. Without fix the test will simply run incorrect code.
12-08-2021

The test does not trigger crash. You need to run test with -XX:+PrintEliminateLocks to see unbalanced Locks/Unlocks eliminations.
12-08-2021

Tried to verify using provided 'compiler/locks/TestNestedLocksElimination.java' against jdk-17+26 (pre-fix), but the test doesn't fail. Closing as 'Not verified'
12-08-2021

11u Fix Request Backporting this patch eliminates a crash. Patch does not apply cleanly to 11u. 11u RFR: https://github.com/openjdk/jdk11u-dev/pull/115
15-07-2021

I attached final patch 8268347_11u_final.patch pushed into Oracle JDK 11u. And patch for JDK 8u.
17-06-2021

> It could be the reason (I hope) for other failures related to unbalanced locks because call stacks show C2 compiled methods: > JDK-8066576 : Lock still held Seems unlikely given that was an issue in 8 and we did not see this issue in 8. Also it would require a different manifestation of the elimination bug where we elide the unlock but not the lock (which is the opposite of the known problem) - is that possible? > JDK-8071813 : assert(false) failed: Non-balanced monitor enter/exit! Likely JNI locking Possibly but hard to tell. > JDK-8222313 : Jvm crashes while on wait Yes - now closed as a duplicate of this bug. It was the same library being used.
15-06-2021

Changeset: 4d8b5c70 Author: Vladimir Kozlov <kvn@openjdk.org> Date: 2021-06-14 23:41:50 +0000 URL: https://git.openjdk.java.net/jdk17/commit/4d8b5c70dff51470210a0ca93b932af1b27c9f27
14-06-2021

Running the test with JDK8 shows also unbalanced (7) eliminations: ***Eliminated 1 unlocks and 1 locks <<<. in this line "Eliminated" means "Coarsened" - locks are eliminated late with following lines. ++++ Eliminated: 794 Lock ++++ Eliminated: 789 Unlock ++++ Eliminated: 783 Lock ++++ Eliminated: 285 Unlock ++++ Eliminated: 203 Lock ++++ Eliminated: 179 Unlock ++++ Eliminated: 162 Lock
12-06-2021

JDK-7125896 "Eliminate nested locks" was pushed into JDK 8. It could be affect and we may need to backport the fix there too.
11-06-2021

It could be the reason (I hope) for other failures related to unbalanced locks because call stacks show C2 compiled methods: JDK-8066576 : Lock still held JDK-8071813 : assert(false) failed: Non-balanced monitor enter/exit! Likely JNI locking JDK-8222313 : Jvm crashes while on wait
11-06-2021

LW=HMM=P2 Impact = High, due to crash Likelihood = Medium, Not seen very often, but there are few reports that observed the same issue Workaround = Medium, use -XX:-EliminateLocks/EliminateNestedLocks flags
07-06-2021