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

Kerem Kat krk at openjdk.org
Tue Sep 9 09:30:27 UTC 2025


On Mon, 8 Sep 2025 09:40:58 GMT, Kevin Walls <kevinw at openjdk.org> wrote:

>> 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`.
>
> Have used these occasionally and am surprised to realize psf and others show frames from the VM operation's caller thread.
> 
> 
> The duplication around the active() checks is odd.  If we are adding:
> 
> bool onThread()
> If no current thread, print warning and return false.  Callers will return immediately.
> 
> Could we add e.g.:
> 
> bool hasActiveThread()
> If no current thread, print warning and return false.  Callers will return immediately.
> If no active thread, print warning and return false.  Callers will return immediately.
> Callers can go on to use  JavaThread* p = JavaThread::active(); without checking (as we currently do).
> (We are presumably in a debugger session, speed is not the priority, repeating some checks or fetching of active() is not tragic, but this file has less duplication.)

Thank you for the description for `hasActiveThread`. Is it much different than the `onJavaThread` in the first revision?

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

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


More information about the hotspot-dev mailing list