On Thu, 11 Dec 2025 22:07:45 GMT, Patricio Chilano Mateo
<[email protected]> wrote:
>> Serguei Spitsyn has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> review: restored the assert and fixed the issue caused it to fire
>
> src/hotspot/share/runtime/handshake.cpp line 524:
>
>> 522: assert(allow_suspend || !check_async_exception, "invalid case");
>> 523: #if INCLUDE_JVMTI
>> 524: if (allow_suspend && (_handshakee->is_disable_suspend() ||
>> _handshakee->is_vthread_transition_disabler())) {
>
> I see we need this because a thread could be suspended outside a disable
> operation (e.g. waiting in `disable_transition_for_one()`) but only process
> the suspend once inside it. Looking at the places where we use
> `MountUnmountDisabler`, the one where I can clearly see this could happen is
> `JvmtiEnv::InterruptThread` because of the upcall to Java. Did you encounter
> any other places where we could process suspend requests inside a disabling
> operation?
Not sure, I correctly understand your question. Are you asking about possible
suspend points inside a `MountUnmountDisabler` scope? It is used for
implementation of the JVMTI functions which have a deal with threads. They
normally have suspend points (points where an asynchronous suspend operation
can be picked and executed by the target thread itself with
`HandshakeState::process_by_self()`), for instance, because they use handshakes.
As I understand, the `ThreadSelfSuspensionHandshakeClosure` is an
`AsyncHandshakeClosure`. As the suspending operation is asynchronous, the
suspending thread can finish its suspending work and get out of the exclusive
operation scope. Then the target thread can enter its own
`MountUnmountDisabler` scope and get suspended there. The tweak above is to
disallow such a scenario.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/28740#discussion_r2613932869
