On Sun, 25 May 2025 13:11:41 GMT, Chen Liang <li...@openjdk.org> wrote:
>> This is first (hotspot) part of the update for
>> `HotSpotDiagnosticMXBean.dumpThreads` and `jcmd Thread.dump_to_file` to
>> include lock information in thread dumps (JDK-8356870).
>> The update has been split into parts to simplify reviewing.
>> The fix contains an implementation of `jdk.internal.vm.ThreadSnapshot` class
>> to gather required information about a thread.
>> Second (dependent) part includes changes in
>> `HotSpotDiagnosticMXBean.dumpThreads`/`jcmd Thread.dump_to_file`, spec
>> updates and tests for the functionality.
>>
>> Testing: new `HotSpotDiagnosticMXBean.dumpThreads`/`jcmd
>> Thread.dump_to_file` functionality was tested in loom repo;
>> sanity tier1 (this fix only)
>
> src/hotspot/share/classfile/javaClasses.cpp line 5519:
>
>> 5517: oop
>> java_util_concurrent_locks_AbstractOwnableSynchronizer::get_owner_threadObj(oop
>> obj) {
>> 5518: assert(_owner_offset != 0, "Must be initialized");
>> 5519: return obj->obj_field_acquire(_owner_offset);
>
> We might split this into a separate patch if this affects existing usages in
> threadService.cpp.
The pre-existing usage is from a VM operation so I don't think there is an
existing issue. The new usage is in handshake operation and this field is on
the ownable synchronizer rather than the thread so need to change it.
> src/java.base/share/classes/jdk/internal/vm/ThreadSnapshot.java line 33:
>
>> 31: * Represents a snapshot of information about a Thread.
>> 32: */
>> 33: class ThreadSnapshot {
>
> All class declarations here should be made final (except enum and record
> which are already implicitly final)
This is a JDK internal class, it doesn't really need to be final but it should
have a private constructor to at least avoid something in this internal package
from creating one directly.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/25425#discussion_r2107753356
PR Review Comment: https://git.openjdk.org/jdk/pull/25425#discussion_r2107754207
