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

[David Holmes]

On Thu, 18 Sep 2025 03:13:01 GMT, Leonid Mesnik <lmesnik at openjdk.org> wrote:

>> src/hotspot/share/runtime/suspendResumeManager.cpp line 84:
>> 
>>> 82: }
>>> 83: 
>>> 84: void SuspendResumeManager::set_suspended_current_thread(int64_t vthread_id, bool register_vthread_SR) {
>> 
>> It was totally not at all apparent that `_target` had to be the current thread here! Is it always the current thread? Won't this variant only ever get called when `register_vthread_SR` would be true - in which case we don't need that parameter?
>
> Yes, the purpose of this specialized method is to "pre-load"`thread_id = _target->vthread()` before current thread became blocked.    It is not planned to use this method for any other purpose.
> The `set_suspended_current_thread` is called after check
> ` if (_target == self) {` in `void SuspendResumeManager::set_suspended_current_thread(int64_t vthread_id, bool register_vthread_SR) {`
> 
> 
> And the value 'register_vthread_SR'  doesn't correlate with this method. It depends on which jvmti method is used : 
>  - true for SuspenAllVirutalThrreads 
>  - false for SuspendThread/SuspendThreadList
>  In both cases we can try to suspend current thread. So this method is called in both cases, even thread_id is really used only for SuspenAllVirutalThrreads.
> 
> Hope it makes clearer.

Thanks - though I find this code rather convoluted. We now always extract the `id` even though we only need it in the `register_vthread_SR == true` case. I tried different re-arrangements of the code to see if it could be cleaner, but the JVMTI part is just messy.

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

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