RFR: 8334215: serviceability/dcmd/thread/PrintMountedVirtualThread.java failing with JTREG_TEST_THREAD_FACTORY=Virtual

[David Holmes]

On Mon, 17 Jun 2024 09:13:57 GMT, Inigo Mediavilla Saiz <duke at openjdk.org> wrote:

> Follow up to https://github.com/openjdk/jdk/pull/19482 that was causing issues when the PrintMountedVirtualTest.java was
> running with `JTREG_TEST_THREAD_FACTORY=Virtual` in the loom repo. 
> 
> - Fixes issues where the test observes the thread during transitions.
> - Fixes a potential issue in the test where CountDownLatch.countDown unparks the main (virtual) thread and the main thread observes the dummy thread is transition .

src/hotspot/share/runtime/threads.cpp line 1334:

> 1332:           if (p->is_vthread_mounted()) {
> 1333:             const oop vt = p->vthread();
> 1334:             if (vt != thread_oop) {

We need a comment explaining why we have to check for this as without knowing the implementation quirks it seems non-sensical for a thread to have mounted itself!

src/hotspot/share/runtime/threads.cpp line 1335:

> 1333:             const oop vt = p->vthread();
> 1334:             if (vt != thread_oop) {
> 1335:               assert(vt != nullptr, "vthread should not be null when vthread is mounted");

I think the assert still belongs in the original position doesn't it? Or could the problem we hit here cause a transient null to appear as well?

test/hotspot/jtreg/serviceability/dcmd/thread/PrintMountedVirtualThread.java line 43:

> 41:     public void run(CommandExecutor executor) throws InterruptedException {
> 42:         var shouldFinish = new AtomicBoolean(false);
> 43:         var started = new AtomicBoolean();

Not sure how this change make things better? Why would we want to sleep and poll rather than park until signalled?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/19744#discussion_r1642745542
PR Review Comment: https://git.openjdk.org/jdk/pull/19744#discussion_r1642744456
PR Review Comment: https://git.openjdk.org/jdk/pull/19744#discussion_r1642747708