On Tue, 1 Jul 2025 03:01:24 GMT, David Holmes <dhol...@openjdk.org> wrote:
>> Kevin Walls has updated the pull request with a new target base due to a >> merge or a rebase. The incremental webrev excludes the unrelated changes >> brought in by the merge/rebase. The pull request contains 13 additional >> commits since the last revision: >> >> - remove test definition changes >> - TLH: use cv_internal_thread_to_JavaThread() >> - Merge remote-tracking branch 'upstream/master' into 8359870_threadexited >> - Test requires: permit linux debug testing >> - comment update >> - comment update >> - newline >> - Test fails on minimal VM: require jvmti feature >> - Correct THROW macro >> - ThreadDumper thread count >> - ... and 3 more: https://git.openjdk.org/jdk/compare/88ec329a...e2043438 > > src/hotspot/share/services/threadService.cpp line 1450: > >> 1448: >> 1449: if (!has_javathread && !is_virtual) { >> 1450: return nullptr; // thread terminated > > Suggestion: > > return nullptr; // thread terminated so not of interest > > It took me a lot of backtracking in the Java code to find ThreadContainer > which finally stated that only live threads are of interest, so an expanded > comment here would help me. ( I was ready to suggest we should be creating a > `ThreadSnapshot` that represents the terminated thread.) Yes, although we could return _something_ for a terminated thread, that seems like a fiction that might do us no good, forgetting it and moving on seems good... ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/25958#discussion_r2176995651
