RFR: 8299777: Test runtime/NMT/BaselineWithParameter.java timed out

[David Holmes]

On Mon, 20 Feb 2023 13:46:46 GMT, Stefan Karlsson <stefank at openjdk.org> wrote:

> I found this issue while debugging a hang in runtime/NMT/JcmdSummaryClass.java. We have the same issue in a few other NMT tests, and the BaselineWithParameter.java is one of them.
> 
> The common pattern among these tests is that they have one main process that forks a jcmd process against itself. The forked jcmd process writes its output to stdout (and maybe stderr). The problematic part of these tests is that the main process doesn't drain the streams of the forked jcmd before it calls waitFor(). This usually works and the tests passes, and I guess that's because there's some internal buffer that the jcmd process can write to without blocking. However, if too much data gets written the process gets blocked and the test times out. I can reproduce this by artificially adding more output to the NMT jcmd with the following patch
> 
> 
> diff --git a/src/hotspot/share/services/nmtDCmd.cpp b/src/hotspot/share/services/nmtDCmd.cpp
> index f64c65c2dc8..42dbe6c1ae2 100644
> --- a/src/hotspot/share/services/nmtDCmd.cpp
> +++ b/src/hotspot/share/services/nmtDCmd.cpp
> @@ -108,6 +108,11 @@ void NMTDCmd::execute(DCmdSource source, TRAPS) {
>    // Serialize NMT query
>    MutexLocker locker(THREAD, MemTracker::query_lock());
>  
> +  // Fill up the output
> +  for (int i = 0; i < 8 * 1024; i++) {
> +    output()->print_cr("Fake line: %d", i);
> +  }
> +
>    if (_summary.value()) {
>      report(true, scale_unit);
>    } else if (_detail.value()) {
> 
> 
> This is the problematic test pattern:
> 
> // Set up command line
> pb.command(new String[] { JDKToolFinder.getJDKTool("jcmd"), pid, "VM.native_memory"});
> 
> // Test hangs here waiting for the jcmd process to finish,
> // which it won't because nothing accepts its written output.
> pb.start().waitFor();
> 
> // Verify the output - (secondary bug here, see comments below)
> output = new OutputAnalyzer(pb.start());
> 
> 
> The correct way to write these tests is simply to remove the waitFor call and let the OutputAnalyzer deal with waiting for the forked process to finish:
> 
> // Set up command line
> pb.command(new String[] { JDKToolFinder.getJDKTool("jcmd"), pid, "VM.native_memory"});
> 
> // Start the process and verify the output
> output = new OutputAnalyzer(pb.start());
> 
> 
> The secondary problem with this tests is that they forks the jcmd process twice, via two calls to pb.start(). This isn't causing the hangs, but we shouldn't be doing that.
> 
> I've tested this fix by running the tests in runtime/NMT with and without the nmtDcmd.cpp patch above.

Fixup looks good. Good catch.

Thanks.

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

Marked as reviewed by dholmes (Reviewer).

PR: https://git.openjdk.org/jdk/pull/12663