[crac] RFR: 8368570: [CRaC] Improve JdwpTransportTest

[Timofei Pushkin]

On Wed, 24 Sep 2025 15:18:35 GMT, Roman Marchenko <rmarchenko at openjdk.org> wrote:

> JdwpTransportTest sometimes hangs, so it'd be good to dump Java threads, and create a corefile if possible.
> 
> Also improved reading from output stream.

test/jdk/jdk/crac/jdwp/JdwpTransportTest.java line 2:

> 1: /*
> 2:  * Copyright (c) 2023-2025, Azul Systems, Inc. All rights reserved.

I believe the convention is to write it like this, not sure if there are any legally significant differences
Suggestion:

 * Copyright (c) 2023, 2025, Azul Systems, Inc. All rights reserved.

test/jdk/jdk/crac/jdwp/JdwpTransportTest.java line 112:

> 110:         CracProcess process = builder.captureOutput(true).startCheckpoint();
> 111:         var errReader = new AsyncStreamReader(process.errOutput());
> 112:         var reader = new AsyncStreamReader(process.output());

Not that it matters much in this test, but I believe `AsyncStreamReader` should be made `AutoCloseable` (on close, close the buffer, maybe also stop the reading thread) and these should be treated as resources in the try below

test/jdk/jdk/crac/jdwp/JdwpTransportTest.java line 150:

> 148:             System.err.println("reader.isRunning()=" + reader.isRunning());
> 149:             CracProcess.printThreadDump(process.pid());
> 150:             CracProcess.dumpProcess(process.pid());

If we end up always taking the PID from `CracProcess` itself why making these two static?

test/jdk/jdk/crac/jdwp/JdwpTransportTest.java line 168:

> 166:     public void exec() throws Exception {
> 167:         System.out.println(STARTED + ", pid=" + ProcessHandle.current().pid());
> 168:         System.out.flush();

AFAIK `System.out` is auto-flushed when `\n` is written, so `println` should be sufficient

test/lib/jdk/test/lib/crac/AsyncStreamReader.java line 1:

> 1: package jdk.test.lib.crac;

Missing copyright header

test/lib/jdk/test/lib/crac/CracProcess.java line 236:

> 234:     }
> 235: 
> 236:     public static void dumpProcess(long pid) throws IOException, InterruptedException {

`JdwpTransportTest` runs on all platforms and I believe this is not going to work on non-Linux

test/lib/jdk/test/lib/crac/CracProcess.java line 254:

> 252:             }
> 253:             if (exitCode == 0) {
> 254:                 System.out.println("Core dump seems created successfully for pid=" + pid);

Suggestion:

                System.out.println("Core dump seems to be created successfully for pid=" + pid);

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

PR Review Comment: https://git.openjdk.org/crac/pull/265#discussion_r2387173852
PR Review Comment: https://git.openjdk.org/crac/pull/265#discussion_r2387200475
PR Review Comment: https://git.openjdk.org/crac/pull/265#discussion_r2387272950
PR Review Comment: https://git.openjdk.org/crac/pull/265#discussion_r2387268144
PR Review Comment: https://git.openjdk.org/crac/pull/265#discussion_r2387195030
PR Review Comment: https://git.openjdk.org/crac/pull/265#discussion_r2387311754
PR Review Comment: https://git.openjdk.org/crac/pull/265#discussion_r2387314760