RFR: 8366154: Validate thread type requirements in debug commands [v2]

Kevin Walls kevinw at openjdk.org
Mon Sep 8 09:57:11 UTC 2025


On Tue, 2 Sep 2025 10:08:58 GMT, Kerem Kat <krk at openjdk.org> wrote:

>> Prevents segmentation faults during `gdb` sessions. The crashes were caused by the `ResourceMark` constructor being called on a native thread, which is not supported. This happened when invoking debug commands that require a `Thread` or `JavaThread` context from an incorrect thread type.
>> 
>> ### Solution
>> 
>> This change introduces `onThread()` and `onJavaThread()` helper methods to the `Command` class. These methods validate the thread context and ensure `ResourceMark` is only created when on a valid VM thread. All thread-dependent debug commands now use these guards to validate the context, printing a clear error and exiting gracefully upon failure.
>> 
>> ### Testing
>> 
>> Manually verified using `gdb` by calling the modified commands (`ps`, `universe`, `pns`, etc.) from different thread contexts (native, Java, and non-java threads) to ensure they fail gracefully with an error message instead of crashing the debug session.
>
> Kerem Kat has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - add include for global placement new
>  - remove onJavaThread and check JavaThread::active null where needed

src/hotspot/share/utilities/debug.cpp line 308:

> 306:  public:
> 307:   Command(const char* str) : _has_rm(false) {
> 308:     if (level++ == 0) {

"level" can recognise concurrent usage, but doesn't protect against it?  The commands still proceed to do their thing, so it doesn't seem right make us skip printing "Executing..." if level was not exactly zero?

(Adding the flush() if there is concurrent might be a good thing, it may help if there is output that could be confused, if that is even possible to achieve...)

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

PR Review Comment: https://git.openjdk.org/jdk/pull/27033#discussion_r2329749851


More information about the hotspot-dev mailing list