On Mon, 27 Apr 2026 19:15:50 GMT, Patricio Chilano Mateo 
<[email protected]> wrote:

>> src/hotspot/share/services/threadService.cpp line 1476:
>> 
>>> 1474:   }
>>> 1475: 
>>> 1476:   assert(cl._thread_status != JavaThreadStatus::NEW, "unstarted 
>>> Thread");
>> 
>> I think the intent of this comment was to check that the handshake actually 
>> executed and advanced the initial state from NEW, but now it is impossible 
>> to be set to NEW because it cannot encounter an unstarted thread.
>> If the thread was unstarted then `has_javathread` would be false and we 
>> would return at line 1464 (though the comment is wrong).
>> 
>> As you note the Handshake is not guaranteed to execute as we are racing with 
>> termination so the assert was invalid anyway.
>
>> I think the intent of this comment was to check that the handshake actually 
>> executed and advanced the initial state from NEW, but now it is impossible 
>> to be set to NEW because it cannot encounter an unstarted thread.
>>
> I added a new assert to verify the handshake was actually executed, except 
> possibly for the already terminated platform thread case.
> 
>> If the thread was unstarted then has_javathread would be false and we would 
>> return at line 1464 (though the comment is wrong).
>>
> Note that there is a `isAlive` check in `ThreadSnapshot.of` before calling 
> this VM method: 
> https://github.com/openjdk/jdk/blob/5ddb0fd10858106f508d2f2c9b6f64d9ea094de5/src/java.base/share/classes/jdk/internal/vm/ThreadSnapshot.java#L63-L65
> 
> For platform threads, that checks for `eetop != 0`, which is set at the end 
> of `JavaThread::prepare` after the `JavaThread` was added to the list. So the 
> only way for `has_javathread` to be false there is if the thread already 
> terminated.

That combined with the new `_processed` flag just makes the assert for !NEW 
even more redundant. I think you can just delete that assertion as it was a 
proxy for the new assertion, and you can restore the default status to NEW 
instead of TERMINATED.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/30903#discussion_r3151998643

Reply via email to