RFR: 8357650: ThreadSnapshot to take snapshot of thread for thread dumps [v2]

[Alex Menkov]

On Wed, 28 May 2025 07:09:06 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Alex Menkov has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   move to ThreadService
>
> src/hotspot/share/services/threadService.cpp line 1189:
> 
>> 1187:     _thread_status(), _name(nullptr),
>> 1188:     _locks(nullptr), _blocker(), _blocker_owner(nullptr) { }
>> 1189:   virtual ~GetThreadSnapshotClosure() {
> 
> Suggestion:
> 
>     _locks(nullptr), _blocker(), _blocker_owner(nullptr) { 
>   }
>     
>   virtual ~GetThreadSnapshotClosure() {

Fixed

> src/hotspot/share/services/threadService.cpp line 1434:
> 
>> 1432:   static Handle allocate(InstanceKlass* klass, TRAPS) {
>> 1433:     init(klass, CHECK_NH);
>> 1434:     return klass->allocate_instance_handle(CHECK_NH);
> 
> Suggestion:
> 
>     return klass->allocate_instance_handle(CHECK_NH);
> 
> Suggestion:
> 
>     Handle h_k = klass->allocate_instance_handle(CHECK_NH);
>     return h_k;
> 
> You can't use `CHECK` on a `return` statement.

Thanks, fixed

> src/hotspot/share/services/threadService.cpp line 1528:
> 
>> 1526:   // StackTrace
>> 1527:   InstanceKlass* ste_klass = vmClasses::StackTraceElement_klass();
>> 1528:   assert(ste_klass != nullptr, "must be loaded in 1.4+");
> 
> Suggestion:
> 
>   assert(ste_klass != nullptr, "must be loaded");
> 
> The 1.4+ is not relevant these days

done

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25425#discussion_r2113109592
PR Review Comment: https://git.openjdk.org/jdk/pull/25425#discussion_r2113110231
PR Review Comment: https://git.openjdk.org/jdk/pull/25425#discussion_r2113110556