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

[David Holmes]

On Thu, 4 Sep 2025 19:35:36 GMT, Paul Hohensee <phh at openjdk.org> wrote:

>> src/hotspot/share/utilities/debug.cpp line 341:
>> 
>>> 339:     }
>>> 340:     return true;
>>> 341:   }
>> 
>> I don't think we need this. The commands that require a JavaThread should be checking that directly themselves. Typically we assume/expect the person debugging to know what they are dealing with and use the appropriate commands.
>
> @dholmes-ora, are you sure you want this level of code duplication? As is it, the latest version duplicates the following 3 times
> 
>  if (!c.onThread()) return;
>   JavaThread* p = JavaThread::active();
>   if (p == nullptr) {
>     tty->print_cr("Failed: JavaThread::active is null");
>     return;
>   }
>   tty->print(" for thread: ");
>   p->print();
>   tty->cr();

It is not ideal but the original

 if (!c.onJavaThread()) return;
  JavaThread* p = JavaThread::active();

is also duplicating the internals both inside `isOnJavaThread()` and in the callers. And what is mainly tripping us up is that `active()` is allowed to return null, but this fix wants to detect a null current thread as a sign of being detached. So we end up calling `Thread::current` multiple times. All of the debug functions that use `active()` are currently broken anyway because they never check it for null (and I'm not even sure they truly expect to work on a delegated JavaThread??). So I see this fix as having two parts:
1. The `onThread` check takes care of being attached
2. The `active() == nullptr` check just fixes the current broken code.

But I still dislike the internal duplication between `onThread` and `active`.

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

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