On Wed, 8 Jul 2026 05:39:11 GMT, David Holmes <[email protected]> wrote:
>>>So even though this case has not been observed to fail we are now making it >>>fail "just in case"? That seems wrong. >> I would consider it a perfectly legitimate debugging scenario to have a >> thread blocked on monitor entry, suspend it (as required) and call >> StopThread on it, and expect it to work. >>> >> Adding to Alan's comment, note that for unmounted virtual threads >> `StopThread` returns `JVMTI_ERROR_OPAQUE_FRAME`, so that scenario is already >> not supported. The reason why the test expects it to work is because it’s >> pinning the vthreads. The other case where today that works is if the >> `StopThread` is sent at the `MonitorContendedEnter` event, because that has >> the effect of pinning the vthread. >> >> AFAIU the use case for this is testing in the debugger how the code behaves >> when a specific exception is thrown. Setting a breakpoint at a synchronized >> block/method (before monitorenter is executed) or immediately after, and >> then asking the debugger to throw an exception once it hits the breakpoint, >> should work fine. In fact, kill003.java is testing this latter scenario and >> there are no issues. >> >> Test kill001.java on the other hand, expects `StopThread` to succeed while >> the target vthread is blocked in the VM monitorenter call. Ignoring that it >> already doesn’t work if the virtual thread is not pinned, we don’t throw >> exceptions when returning from the monitorenter call. So even if we allow >> `StopThread` to succeed, the async exception will only be processed and >> thrown at some later bytecode (ideally the immediate next one). This would >> be similar to setting a breakpoint right after monitorenter and throwing the >> exception there (which works fine). The problem with allowing the exception >> to be deferred to some later bytecode is that we don’t know where that will >> be. So we could construct a case where we hit the same assert as in the >> reported bug. >> >> If we still want to support this case, as mentioned in the PR description I >> experimented briefly with an alternative version that allows async exception >> deferral. Here is a draft patch: >> https://github.com/pchilano/jdk/compare/JDK-8386116-alt-draft >> The idea is that if we reach a start transition without the exception being >> processed still, we save it in the `JvmtiThreadState` of the vthread when >> unmounting, and then we throw it only after the vthread is ready to resume >> execution. It still doesn’t support the unmounted case (shouldn’t be that >> hard to add). But this is more complicated and I’m not sure it’s worth it. > > If the vthread is pinned then we cannot hit any of the problematic vthread > code - correct? In which case the vthread case would look just like the > platform thread case, and so if it works today then it should keep working. > Even if we are at the no-async monitorenter it should be safe to defer > processing of the async exception to the next bytecode as that can't be a > bytecode in critical vthread code. We could still hit it because a call to `Thread.yield` after grabbing the monitor would still reach the unmount logic, i.e. `yieldContinuation`. The exception might be processed on the return from `startTransition`. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/31759#discussion_r3547483888
