RFR: 8316682: serviceability/jvmti/vthread/SelfSuspendDisablerTest timed out [v4]

[Patricio Chilano Mateo]

On Thu, 10 Apr 2025 06:41:42 GMT, Serguei Spitsyn <sspitsyn at openjdk.org> wrote:

>> This fixes the issue with lack of synchronization between JVMTI thread suspend and resume functions in a self-suspend case. More detailed fix description is in the first PR comment.
>> 
>> Testing: Ran mach5 tiers 1-6.
>
> Serguei Spitsyn has updated the pull request incrementally with one additional commit since the last revision:
> 
>   review: added general comment about sync between suspend_thread and resume_thread

Hi Serguei, 

The fix looks correct but I think we could probably do something that avoids this overhead and also simplifies the code.
Looking at each suspend case we have, in terms of lack of sync between self-suspend and resume:
- unmounted vthread: should not be an issue, unmounted implies it’s not self suspend.
- platform thread (not carrying vthread): Currently we have an issue because we are setting `_carrier_thread_suspended` always, not only for the carrying vthread case. We then assume that `_carrier_thread_suspended`=true implies suspension, but that will only be the case for the carrying vthread case which checks for this and suspends in `finish_VTMS_transition`. A normal platform thread needs to still call handshake code to suspend. This is the issue with this test because a resumer sees the target is already suspended and calls resume before the target actually calls handshake suspend. We should just set `_carrier_thread_suspended` on the carrying vthread case.
- platform thread carrying vthread: we can keep using `_carrier_thread_suspended` but should use atomic cmpxchg when updating.
- mounted vthread: Currently we have an issue because we are doing two things that are not atomic, adding/removing to the list and calling handshake code to suspend/resume. We could move the update of the list inside the handshake instead.

I’ve been experimenting with the above changes here:  https://github.com/pchilano/jdk/commit/7cce48ef14bdb977b9c65a33370144f3d61fc974
Notice that the `suspend_thread/resume_thread` methods are simpler to read because the cases are clearly separated. This would also fix the lack of sync between self-suspend and suspend, which we still have if we just change resume to be a handshake. What do you think? The `JvmtiVTMSTransitionDisabler` changes could be done in a separate fix.

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

PR Comment: https://git.openjdk.org/jdk/pull/24269#issuecomment-2840140925