On Thu, 18 Dec 2025 23:25:17 GMT, Patricio Chilano Mateo <[email protected]> wrote:
>> So this `_is_disable_suspend` flag will prevent the target from processing >> the async handshake and suspend, but the suspender will still consider the >> target suspended once `SuspendThreadHandshakeClosure` is done. We would need >> to check the state of the target and don't consider it "handshake safe" if >> it's in a JNI critical region. > >> Thanks for looking at this @pchilano >> >> > So this `_is_disable_suspend` flag will prevent the target from processing >> > the async handshake and suspend, but the suspender will still consider the >> > target suspended once `SuspendThreadHandshakeClosure` is done. >> >> This two-step process always causes me confusion. So the "disabling" >> mechanism is not actually disabling anything from the requesters point of >> view. I don't understand what it is actually doing then? And I think I would >> like it called something else. >> > It was added to prevent deadlocks while executing some critical sections in > methods of the `VirtualThread` class. Don't remember if there was an issue > with delaying the suspender instead. > >> > We would need to check the state of the target and don't consider it >> > "handshake safe" if it's in a JNI critical region. >> >> So rather than "just" disabling suspension whilst in JNI critical your >> suggestion would delay all handshakes and safepoints which seems a far more >> risky proposition. >> > It would only be for suspend operations, as the PR is trying to address this > specific issue with the debugger. I see that something like this was already > suggested in the JBS comments for 8369515, and that there is an alternative > suggestion to return a copy of the object. In any case my comment was mainly > to point out we are not disabling suspension as intended and describe what > would need to be done instead (or at least one of the two possible solutions). @pchilano is the latest proposal more what you were thinking? It has an obvious flaw if the critical region is badly behaved, but perhaps we just live with that. ------------- PR Comment: https://git.openjdk.org/jdk/pull/28884#issuecomment-3673697095
