On Mon, 6 Jan 2025 16:41:21 GMT, Patricio Chilano Mateo 
<pchilanom...@openjdk.org> wrote:

> Please review the following fix. In method 
> `JvmtiEventControllerPrivate::recompute_thread_enabled()`, we are missing to 
> call `leave_interp_only_mode()` for the case where `should_be_interp` is 
> computed as false and `state->is_pending_interp_only_mode()` is true. I added 
> the full trace leading to the crash in the bug comments.
> In JDK-8338383 I removed this assert because the branch condition changed and 
> it became sort of a redundant check. But given that it was able to find this 
> issue I added it back.
> I was able to reproduce the crash easily by adding an extra delay before the 
> assert. I verified the crash doesn’t reproduce anymore with this fix. I also 
> run the patch through mach5 tiers 1-7.
> 
> Thanks,
> Patricio

The fix looks good. It is a great and important finding - thanks!
What was bothering me is that the `EnterInterpOnlyModeClosure` is used in the 
`JvmtiEventControllerPrivate::enter_interp_only_mode()` but no 
`HandshakeClosure` is used in the 
`JvmtiEventControllerPrivate::leave_interp_only_mode()`. The handshake use for 
the `enter_interp_only_mode()` looks like a paranoid overkill to me. It can be 
I just forgot the exact reason why it is used there.

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

Marked as reviewed by sspitsyn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/22931#pullrequestreview-2536049299

Reply via email to