On Tue, 28 Apr 2026 06:24:53 GMT, David Holmes <[email protected]> wrote:
>>> 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. That would be a bit clearer, and would be nicer if it could return nullptr if !cl.processed or the target threadhas the TERMINATED state. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/30903#discussion_r3154427593
