On Fri, 11 Sep 2026 00:25:25 GMT, Patricio Chilano Mateo
<[email protected]> wrote:
>> Denghui Dong has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> add test
>
> src/hotspot/share/services/threadService.cpp line 920:
>
>> 918:
>> 919: oop vthread = thread->vthread();
>> 920: if (vthread != nullptr && vthread != threadObj) { // ThreadSnapshot
>> only captures platform threads
>
> While stress testing the changes with the extra assert, I found a few
> failures with `-XX:+CheckUnhandledOops` due to now taking this branch for the
> platform thread case. Turns out the call to
> `check_for_dangling_thread_pointer` in
> `ThreadService::get_current_contended_monitor` above, acquires the
> `Threads_lock` and that sets `threadObj` to `BAD_OOP_ADDR`, so the oop
> comparison fails. This causes test `SuspendResume3.java` to timeout for
> example. The old code didn’t suffer from this because of the
> `is_vthread_mounted()` check. Simplest fix would be to reload the `threadObj`
> oop. Or maybe better just use `java_lang_VirtualThread::is_instance(vthread)`
> instead of comparing oops, which is easier to read (already has null check).
Fixed. Thanks for the explanation.
> test/hotspot/jtreg/serviceability/threads/ThreadSnapshotRaceTest.java line 36:
>
>> 34: * @test
>> 35: * @bug 8392031
>> 36: * @summary Make sure that ThreadSnapshot::initialize does not crash JVM
>
> Suggestion:
>
> * @summary Make sure that ThreadSnapshot::initialize does not crash JVM
> * @requires vm.continuations
added.
> test/hotspot/jtreg/serviceability/threads/ThreadSnapshotRaceTest.java line 53:
>
>> 51: counter.decrementAndGet();
>> 52: });
>> 53: if (c >= 20_000_000) {
>
> I got intermittent OOM errors with this value, maybe tune it down to 1M?
updated.
> test/hotspot/jtreg/serviceability/threads/ThreadSnapshotRaceTest.java line 72:
>
>> 70: while (true) {
>> 71: ThreadInfo[] infos = bean.getThreadInfo(ids);
>> 72: assertTrue(infos.length > 0);
>
> Note that when calling `carrierIds(bean)` we still can’t guarantee the first
> virtual thread already started (no FJP workers), so we could hit this assert.
updated.
> test/hotspot/jtreg/serviceability/threads/ThreadSnapshotRaceTest.java line 77:
>
>> 75: consumer.start();
>> 76:
>> 77: Thread.sleep(10_000);
>
> We should join consumer and producer before exiting. You could add volatile
> `stop`, set it to true here, and have the workers loop on that.
updated.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/32788#discussion_r3996445745
PR Review Comment: https://git.openjdk.org/jdk/pull/32788#discussion_r3996447076
PR Review Comment: https://git.openjdk.org/jdk/pull/32788#discussion_r3996447469
PR Review Comment: https://git.openjdk.org/jdk/pull/32788#discussion_r3996447735
PR Review Comment: https://git.openjdk.org/jdk/pull/32788#discussion_r3996448017
