RFR: 8253540: InterpreterRuntime::monitorexit should be a JRT_LEAF function

[Patricio Chilano Mateo]

On Wed, 23 Sep 2020 17:19:25 GMT, Daniel D. Daugherty <dcubed at openjdk.org> wrote:

>> That the monitor has already been unlocked, or is a null stacklock monitor has been already checked in the caller, so
>> the code that makes it a JRT_ENTRY_NO_ASYNC is unnecessary.
>> Making it a JRT_LEAF like the compiled method entries makes it safer. We know it can never safepoint and
>> unintentionally install a async exception.
>> Tested with tier1-6.
>
> src/hotspot/share/interpreter/interpreterRuntime.cpp line 746:
> 
>> 744:   if (elem == NULL || h_obj()->is_unlocked()) {
>> 745:     THROW(vmSymbols::java_lang_IllegalMonitorStateException());
>> 746:   }
> 
> In the case of an unbalanced monitorexit(), you are losing
> the throwing of the IllegalMonitorStateException here.
> At one point, we had at least one test that used JNI MonitorExit()
> to induce an unbalanced monitorexit() and it verified that the
> IllegalMonitorStateException was thrown.

But JNI MonitorExit() calls ObjectSynchronizer::jni_exit() instead and that calls check_owner() which will throw
java_lang_IllegalMonitorStateException() if the thread is not the owner. I agree we might still need to throw
java_lang_IllegalMonitorStateException() here for the release bits though. Maybe we still need to cover the case of the
classfile having wrong bytecodes and there is an extra monitorexit?

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

PR: https://git.openjdk.java.net/jdk/pull/320