RFR: 8360702: runtime/Thread/AsyncExceptionTest.java timed out [v2]

[Patricio Chilano Mateo]

On Thu, 2 Oct 2025 04:03:18 GMT, David Holmes <dholmes at openjdk.org> wrote:

> For AsyncExceptionTest.java I have one issue - see below.
> 
> Looking at AsyncExceptionOnMonitorEnter.java we seem to be making a number of assumptions based on the fact we only have real platform threads:
> 
> 1. An async exception is either thrown before we start the monitorenter, or else after we are blocked (is this true? we never used to be able to unblock a monitor acquisition via stop() because the code will still try to unlock it???)
>
So the stopThread operation will only install the async exception handshake. If the target is blocked on monitorenter (or RawMonitorEnter) the handshake won’t be processed until the target acquires the monitor and transitions back to Java. There is a slight difference here between testWithJavaMonitor and testWithJVMTIRawMonitor. enterRawMonitor is a native method so we will throw the exception on return from it. The call to monitorenter uses JRT_ENTRY_NO_ASYNC which doesn’t process the async exceptions. So for testWithJavaMonitor the exception will be likely thrown during Thread.sleep (don’t see other obvious safepoint polls in the bytecodes).

> 2. We can safely hit `Thread.sleep` with an async exception without breaking anything.
> 
Yes. The stopThread handshake will unpark the target after adding the async handshake. The target will see the async condition on wake up and return to Java where the exception will be thrown.

> Aside:
> 
> > //  Don't stop() worker1 with JVMTI raw monitors since if the monitor is not released worker2 will deadlock on enter
> 
> Pre-existing but surely the rawMonitorExit should be in a finally block to avoid this problem.
> 
Ok, I added it but I had to also restrict sending stopThread to only one time for this case. Otherwise the next async exception could be thrown at the finally block before releasing the monitor, causing the same deadlock. In practice I think this should never happen though, except the first time we resolve the call to `exitRawMonitor`, because we first call `InterpreterRuntime::resolve_from_cache` and the exception could be installed there.
Also although unlikely, the target could have already released the raw monitor when it throws the exception. So in the finally block the call to `RawMonitorExit` would return `JVMTI_ERROR_NOT_MONITOR_OWNER`. The test will not fail but we just print an error message. We could skip it like with destroyRawMonitor.

> test/hotspot/jtreg/runtime/Thread/AsyncExceptionTest.java line 82:
> 
>> 80:     public void internalRun1() {
>> 81:         started = true;
>> 82:         try {
> 
> You need to move the setting of `started` to inside the try block else you could throw outside the range of the catch.

Right, fixed. Although the one in `testWithJVMTIRawMonitor` won’t cause the test to fail I moved it inside the try block too just for consistency.

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

PR Comment: https://git.openjdk.org/jdk/pull/27582#issuecomment-3362871360
PR Review Comment: https://git.openjdk.org/jdk/pull/27582#discussion_r2400006650