From a0421605df9df8af6f27df42b28241ac20bbc0d9 Mon Sep 17 00:00:00 2001 From: Alexey Kuznetsov Date: Tue, 22 Sep 2026 22:37:19 -0400 Subject: [PATCH] Improve diagnostics for hung-test dump collection --- .../plugin/dump/DumpHangedTestPlugin.kt | 72 +++++++++++++------ .../dump/DumpHangedTestIntegrationTest.kt | 52 +++++++++++++- 2 files changed, 101 insertions(+), 23 deletions(-) diff --git a/buildSrc/src/main/kotlin/datadog/gradle/plugin/dump/DumpHangedTestPlugin.kt b/buildSrc/src/main/kotlin/datadog/gradle/plugin/dump/DumpHangedTestPlugin.kt index 9f62521d414..95a2ee21411 100644 --- a/buildSrc/src/main/kotlin/datadog/gradle/plugin/dump/DumpHangedTestPlugin.kt +++ b/buildSrc/src/main/kotlin/datadog/gradle/plugin/dump/DumpHangedTestPlugin.kt @@ -3,6 +3,7 @@ package datadog.gradle.plugin.dump import org.gradle.api.Plugin import org.gradle.api.Project import org.gradle.api.Task +import org.gradle.api.logging.Logger import org.gradle.api.model.ObjectFactory import org.gradle.api.provider.Property import org.gradle.api.provider.Provider @@ -118,23 +119,35 @@ class DumpHangedTestPlugin : Plugin { } .get() - dumpsDir.mkdirs() + if (!dumpsDir.isDirectory && !dumpsDir.mkdirs()) { + throw IOException("Could not create dump directory $dumpsDir") + } - ProcessHandle.current().children() - .filter { it.info().commandLine().getOrElse { "" }.contains("Gradle Test Executor") } - .forEach { process -> - collectDump(dumpsDir, process) + var executorCount = 0 + ProcessHandle.current().children().use { children -> + children.filter { it.info().commandLine().getOrElse { "" }.contains("Gradle Test Executor") } + .forEach { process -> + executorCount++ + collectDump(t, dumpsDir, process) - process.children().forEach { child -> - collectDump(dumpsDir, child) + process.children().use { descendants -> + descendants.forEach { child -> collectDump(t, dumpsDir, child) } + } } - } + } + if (executorCount == 0) { + t.logger.warn("No Gradle test executors found for ${t.path}; attempting all-JVM thread dumps") + } // Just in case collect all thread dumps by using special PID `0`. val allThreadsFile = file(dumpsDir, "all-thread-dumps") - runCmd(Redirect.to(allThreadsFile), "jcmd", "0", "Thread.print", "-l") + runCmd(t.logger, t.path, Redirect.to(allThreadsFile), "jcmd", "0", "Thread.print", "-l") + t.logger.quiet("Finished dump collection for ${t.path}; output directory: $dumpsDir") + } catch (e: InterruptedException) { + Thread.currentThread().interrupt() + t.logger.warn("Dump collection interrupted for ${t.path}", e) } catch (e: Throwable) { - t.logger.warn("Taking dumps failed with error: ${e.message ?: e.javaClass.name}, for ${t.path}") + t.logger.warn("Taking dumps failed for ${t.path}", e) } } @@ -153,21 +166,38 @@ class DumpHangedTestPlugin : Plugin { } private fun runCmd( + logger: Logger, + taskPath: String, redirectTo: Redirect, vararg args: String ) { - val exitCode = ProcessBuilder(*args) - .redirectErrorStream(true) - .redirectOutput(redirectTo) - .start() - .waitFor() - - if (exitCode != 0) { - throw IOException("Process failed: ${args.joinToString(" ")}, exit code: $exitCode") + val command = args.joinToString(" ") + val output = redirectTo.file()?.absolutePath ?: "daemon output" + val start = System.nanoTime() + logger.quiet("Starting dump command for $taskPath: $command; output: $output") + var process: Process? = null + try { + process = ProcessBuilder(*args) + .redirectErrorStream(true) + .redirectOutput(redirectTo) + .start() + val exitCode = process.waitFor() + + if (exitCode != 0) { + throw IOException("Process failed with exit code $exitCode") + } + logger.quiet("Completed dump command for $taskPath in ${TimeUnit.NANOSECONDS.toMillis(System.nanoTime() - start)} ms: $command") + } catch (e: InterruptedException) { + process?.destroyForcibly() + logger.warn("Dump command interrupted for $taskPath: $command; output: $output", e) + throw e + } catch (e: Exception) { + logger.warn("Dump command failed for $taskPath: $command; output: $output", e) } } private fun collectDump( + t: Task, baseDir: File, process: ProcessHandle ) { @@ -176,15 +206,15 @@ class DumpHangedTestPlugin : Plugin { if (process.info().command().getOrElse { "" }.contains("/ibm8")) { // On IBM JDK thread dump can be collected by signaling process with `kill -3`. // It will be writen into `/tmp/javacore.YYYYMMDD.HHMMSS.PID.SEQ.txt - runCmd(Redirect.INHERIT, "kill", "-3", pid) + runCmd(t.logger, t.path, Redirect.INHERIT, "kill", "-3", pid) } else { // Collect heap dump by pid. val heapDumpPath = file(baseDir, "$pid-heap-dump", "hprof").absolutePath - runCmd(Redirect.INHERIT, "jcmd", pid, "GC.heap_dump", heapDumpPath) + runCmd(t.logger, t.path, Redirect.INHERIT, "jcmd", pid, "GC.heap_dump", heapDumpPath) // Collect thread dump by pid. val threadDumpFile = file(baseDir, "$pid-thread-dump", "log") - runCmd(Redirect.to(threadDumpFile), "jcmd", pid, "Thread.print", "-l") + runCmd(t.logger, t.path, Redirect.to(threadDumpFile), "jcmd", pid, "Thread.print", "-l") } } } diff --git a/buildSrc/src/test/kotlin/datadog/gradle/plugin/dump/DumpHangedTestIntegrationTest.kt b/buildSrc/src/test/kotlin/datadog/gradle/plugin/dump/DumpHangedTestIntegrationTest.kt index 78279142b40..20451ddeaeb 100644 --- a/buildSrc/src/test/kotlin/datadog/gradle/plugin/dump/DumpHangedTestIntegrationTest.kt +++ b/buildSrc/src/test/kotlin/datadog/gradle/plugin/dump/DumpHangedTestIntegrationTest.kt @@ -5,6 +5,9 @@ import org.junit.jupiter.api.Assertions.assertFalse import org.junit.jupiter.api.Assertions.assertTrue import org.junit.jupiter.api.Test import org.junit.jupiter.api.assertNotNull +import org.junit.jupiter.api.condition.EnabledOnOs +import org.junit.jupiter.api.condition.OS +import java.io.File class DumpHangedTestIntegrationTest : GradleFixture() { @Test @@ -30,6 +33,8 @@ class DumpHangedTestIntegrationTest : GradleFixture() { val dumps = buildFile("dumps") assertTrue(dumps.exists()) // Assert dumps created. + assertTrue(output.any { it.startsWith("Starting dump command for :test:") && it.contains("GC.heap_dump") }) + assertTrue(output.any { it.startsWith("Completed dump command for :test") && it.contains("Thread.print -l") }) // Assert actual dumps created. val dumpFiles = dumps.list() @@ -37,7 +42,50 @@ class DumpHangedTestIntegrationTest : GradleFixture() { assertNotNull(dumpFiles.find { it.startsWith("all-thread-dumps") }) } - private fun runGradleTest(testSleepMillis: Long): List { + @Test + fun `should report directory failures with stack trace`() { + writeFile("build/dumps", "This file prevents creating the dump directory") + + val output = runGradleTest(testSleepMillis = 25_000) + + assertTrue(output.any { it.contains("Taking dumps failed for :test") }) + assertTrue(output.any { it.contains("java.io.IOException: Could not create dump directory") }) + assertTrue(output.any { it.contains("DumpHangedTestPlugin.takeDump(") }) + } + + @Test + @EnabledOnOs(OS.LINUX, OS.MAC) + fun `should attempt thread dumps after heap dump failure`() { + val jcmd = writeFile( + "bin/jcmd", + """ + #!/bin/sh + if [ "${'$'}2" = "GC.heap_dump" ]; then + echo "Synthetic heap dump failure" >&2 + exit 7 + fi + echo "Synthetic thread dump for PID ${'$'}1" + """ + ) + assertTrue(jcmd.setExecutable(true)) + // Start a separate daemon so subprocess lookup uses this test's PATH. + writeGradleProperties("org.gradle.jvmargs=-DdumpFailureTest=true") + + val output = runGradleTest( + testSleepMillis = 25_000, + env = mapOf("PATH" to "${jcmd.parent}${File.pathSeparator}${System.getenv("PATH")}") + ) + + assertTrue(output.any { it.startsWith("Dump command failed for :test:") && it.contains("GC.heap_dump") }) + assertTrue(output.any { it.contains("java.io.IOException: Process failed with exit code 7") }) + assertTrue(output.any { it.contains("DumpHangedTestPlugin.runCmd(") }) + val dumps = buildFile("dumps").listFiles().orEmpty() + assertTrue(dumps.any { it.name.contains("-thread-dump-") && it.readText().startsWith("Synthetic thread dump") }) + assertTrue(dumps.any { it.name.startsWith("all-thread-dumps-") && it.readText().contains("PID 0") }) + assertTrue(output.any { it.startsWith("Finished dump collection for :test;") }) + } + + private fun runGradleTest(testSleepMillis: Long, env: Map = emptyMap()): List { writeSettings("""rootProject.name = "test-project"""") writeRootProject( @@ -91,6 +139,6 @@ class DumpHangedTestIntegrationTest : GradleFixture() { sourceSet = "test" ) - return run("test", forwardOutput = true).output.lines() + return run("test", env = env, forwardOutput = true).output.lines() } }