RFR: 8367725: Incorrect reading of oop in SuspendResumeManager::suspend while thread is blocked [v9]

[David Holmes]

On Wed, 17 Sep 2025 21:05:26 GMT, Leonid Mesnik <lmesnik at openjdk.org> wrote:

>> src/hotspot/share/runtime/suspendResumeManager.hpp line 63:
>> 
>>> 61: 
>>> 62:   // The specific 'set_suspended' implementation for self suspend.
>>> 63:   void set_suspended_with_id(int64_t id, bool register_vthread_SR);
>> 
>> Suggestion:
>> 
>>   // Sets the suspended state to `to`, applying to vthreads if `register_vthread_SR` is true.
>>   void set_suspended(bool to, bool register_vthread_SR);
>> 
>>   // Sets the suspended state to true, applying to vthreads if `register_vthread_SR` is true.
>>   // Applied to the thread with the given `id` and used when we can't extract the thread oop safely.
>>   void set_suspended_with_id(int64_t id, bool register_vthread_SR);
>> 
>> These comments are far from perfect as it is actually hard to explain exactly what `thread` is being operated on - if any!
>
> Really the comment 
> ```// Sets the suspended state to `to`, applying to vthreads if `register_vthread_SR` is true.```
> looks incorrect to me. One might decide that the method doesn't set suspend state if for vthreads if `register_vthread_SR` is false. While the method always set suspend and might register or not vthreads to support SuspendAllVirtualThreads.

Okay I was trying to understand exactly what these methods were doing but I'm finding it somewhat difficult to explain that. But my point is to document that the first method sets the state to true/false as directed, whilst the second always sets to true. But it gets harder to describe things when the id variant is only needed when `register_vthread_SR` is true.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27317#discussion_r2357369469