RFR: 8320310: CompiledMethod::has_monitors flag can be incorrect [v3]

Jorn Vernee jvernee at openjdk.org
Tue Nov 28 11:00:21 UTC 2023


> Currently, the `CompiledMethod::has_monitors` flag is set when either a `monitorenter` is parsed by C1, and `monitorexit` is parsed by C1 or C2 during method compilation. However, not necessarily every bytecode of a method is parsed, which means that we could miss all `monitorenter`/`monitorexit` byte codes in a method, while it actually does use monitors. This can lead to situations where a thread holds a monitor, but `has_monitors` for all frames is set to `false`, leading to an assertion failure in 'freeze_internal' in continuationFreezeThaw.cpp:
> 
>     assert(monitors_on_stack(current) == ((current->held_monitor_count() - current->jni_monitor_count()) > 0),
>          "Held monitor count and locks on stack invariant: " INT64_FORMAT " JNI: " INT64_FORMAT, (int64_t)current->held_monitor_count(), (int64_t)current->jni_monitor_count());
> 
> The proposed fix is to rely on `Method::has_monitor_bytecodes` to set the `has_monitors` flag when compiling, which is immune to issues where not all byte codes of a method are parsed during compilation. We can follow the pattern established for `has_reserved_stack_access`, which is similar.
> 
> Note that this PR is based on: https://github.com/openjdk/jdk/pull/16416 which disables the assertion. The goal of this PR is to fix the issue, and then re-enable the assertion.
> 
> Testing: Tier 1-4, `java/lang/Thread/virtual/stress/PinALot.java`

Jorn Vernee has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 46 commits:

 - Merge branch 'master' into Has_Monitors
 - fix has_monitors tracking. Re-enable assert
 - add interpreter profiling specific test cases
 - rename ex_handler -> exception_handler
 - fix linux compile
 - Revert "add too_many_traps check"
   
   This reverts commit bee05534777dc2caf10362f66fea90a06705a144.
 - add too_many_traps check
 - Remove has_monitors fix
 - Merge branch 'master' into PruneDeadCatchBlocks
 - Only use ProfileExceptionHandlers
 - ... and 36 more: https://git.openjdk.org/jdk/compare/4bcda602...85b2d662

-------------

Changes: https://git.openjdk.org/jdk/pull/16799/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=16799&range=02
  Stats: 31 lines in 4 files changed: 9 ins; 17 del; 5 mod
  Patch: https://git.openjdk.org/jdk/pull/16799.diff
  Fetch: git fetch https://git.openjdk.org/jdk.git pull/16799/head:pull/16799

PR: https://git.openjdk.org/jdk/pull/16799


More information about the hotspot-dev mailing list