On Tue, 3 Feb 2026 06:53:36 GMT, David Holmes <[email protected]> wrote:
>> We want ThreadSnapshot.of(Thread) to accept a Thread in any state. Existing >> behavior is to return null for platform threads that are not alive. For >> virtual threads it will return a snapshot so we want to change that. The >> ThreadNotAlive test in the PR allows us to test these cases as they are hard >> to demonstrate with the thread dump. >> >> ThreadSnapshot.of(Thread) does not filter out the "not alive" cases. It >> could, in which case ThreadSnapshotFactory::get_thread_snapshot would need >> to assert if called with a new/unstarted thread. The terminating thread case >> would still need to be handled by >> ThreadSnapshotFactory::get_thread_snapshot. For platform threads there is no >> JavaThread so it bails easy. For virtual threads it needs to examine the >> state. Would you prefer if ThreadSnapshot.of(Thread) pre-checked the state >> so that get_thread_snapshot could be guaranteed to never see a new/unstarted >> thread? >> >> Update: I changed ThreadSnapshot.of(Thread) to filter before calling >> get_thread_snapshot, hopefully this will be easier to understand. > > I was assuming/expecting that the top-level code in `ThreadDumper` would > filter out not-alive threads the same way `Thread.getStackTrace` does. You > don't want lower-level code to have to worry about NEW threads, though of > course they still have to deal with races against termination. The proposal is that ThreadSnapshot.of(Thread) can be called with any platform or virtual Thread in any state. With the update, it eagerly tests with isAlive so will filter NEW and already TERMINATED threads. If/when we change Thread.getStackTrace to use ThreadSnapshot then the isAlive check can be dropped from Thread.getStackTrace. The underlying implementation in get_thread_snapshot does not need to deal with the NEW state. There is no need for ThreadDumper to do any additional filtering. The thread streams that it consumes filter Threads that are not alive. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/29461#discussion_r2757854181
