On Mon, 9 Mar 2026 01:55:29 GMT, David Holmes <[email protected]> wrote:
>> Zhengyu Gu has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Remove tailing whitespaces
>
> src/hotspot/share/runtime/threadSMR.cpp line 730:
>
>> 728: }
>> 729: } else if (includes(thread)) {
>> 730: // The thread is protected by this list
>
> It needs to be in the list and not exiting.
If it is not in the list, I am not sure if it is safe to check not exiting -
thread way already be deleted.
> src/hotspot/share/runtime/threadSMR.cpp line 889:
>
>> 887: jlong tid = SharedRuntime::get_java_tid(thread);
>> 888: ThreadIdTable::add_thread(tid, thread);
>> 889: }
>
> I would not move this code. Logically the management of the ThreadIdTable is
> part of ThreadSMR. There must be reason we do delete from other code, but I'd
> be looking at whether that code can move into ThreadSMR rather than moving
> this code out.
I don't mind to revert. `Remove` cannot be done in side `ThreadSMR`, because
at the point, GC barrier has been unattached, it can no longer touch `oop`.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/30105#discussion_r2905280119
PR Review Comment: https://git.openjdk.org/jdk/pull/30105#discussion_r2905296450
