RFR: 8291555: Replace stack-locking with fast-locking

[Robbin Ehn]

On Thu, 11 Aug 2022 11:39:01 GMT, Robbin Ehn <rehn at openjdk.org> wrote:

>>> > Thanks for giving this PR a spin. I pushed a fix for the aarch64 build problem (seems weird that GHA did not catch it).
>>> 
>>> NP, thanks. I notice some other user of owning_thread_from_monitor_owner() such as DeadlockCycle::print_on_with() which asserts on "assert(adr != reinterpret_cast<void*>(1)) failed: must convert to lock object".
>> 
>> Do you know by any chance which tests trigger this?
>
>> > > Thanks for giving this PR a spin. I pushed a fix for the aarch64 build problem (seems weird that GHA did not catch it).
>> > 
>> > 
>> > NP, thanks. I notice some other user of owning_thread_from_monitor_owner() such as DeadlockCycle::print_on_with() which asserts on "assert(adr != reinterpret_cast<void*>(1)) failed: must convert to lock object".
>> 
>> Do you know by any chance which tests trigger this?
> 
> Yes, there is a couple of to choose from, I think the jstack cmd may be easiest: jstack/DeadlockDetectionTest.java

> @robehn or @dholmes-ora I believe one of you mentioned somewhere (can't find the comment, though) that we might need to support the bytecode sequence monitorenter A; monitorenter B; monitorexit A; monitorexit B; properly. I have now made a testcase that checks this, and it does indeed fail with this PR, while passing with upstream. Also, the JVM spec doesn't mention anywhere that it is required that monitorenter/exit are properly nested. I'll have to fix this in the interpreter (JIT compilers refuse to compile not-properly-nested monitorenter/exit anyway).
> 
> See https://github.com/rkennke/jdk/blob/fast-locking/test/hotspot/jtreg/runtime/locking/TestUnstructuredLocking.jasm

jvms-2.11.10

> Structured locking is the situation when, during a method invocation, every exit on a given monitor matches a preceding entry on that monitor. Since there is no assurance that all code submitted to the Java Virtual Machine will perform structured locking, implementations of the Java Virtual Machine are permitted but not required to enforce both of the following two rules guaranteeing structured locking. Let T be a thread and M be a monitor. Then:
> 
> The number of monitor entries performed by T on M during a method invocation must equal the number of monitor exits performed by T on M during the method invocation whether the method invocation completes normally or abruptly.
> 
> At no point during a method invocation may the number of monitor exits performed by T on M since the method invocation exceed the number of monitor entries performed by T on M since the method invocation.
> 
> Note that the monitor entry and exit automatically performed by the Java Virtual Machine when invoking a synchronized method are considered to occur during the calling method's invocation.

I think the intent of above was to allow enforcing structured locking.

In relevant other projects, we support only structured locking in Java, but permit some unstructured locking when done via JNI.
In that project JNI monitor enter/exit do not use the lockstack.

I don't think we today fully support unstructured locking either:

void foo_lock() {
  monitorenter(this);
  // If VM abruptly returns here 'this' will be unlocked
  // Because VM assumes structured locking.
  // see e.g. remove_activation(...)
}


*I scratch this as it was a bit off topic.*

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

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