Skip to content

Commit c096994

Browse files
AlexeyKuznetsov-DDAlexeyKuznetsov-DD
authored
Fixed race condition on dumping future cleanup. (#9607)
1 parent b98b760 commit c096994

File tree

7 files changed

+308
-90
lines changed

7 files changed

+308
-90
lines changed

‎build.gradle.kts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,13 @@ import com.diffplug.gradle.spotless.SpotlessExtension
33
plugins {
44
id("datadog.gradle-debug")
55
id("datadog.dependency-locking")
6+
id("datadog.tracer-version")
7+
id("datadog.dump-hanged-test")
68

79
id("com.diffplug.spotless") version "6.13.0"
810
id("com.github.spotbugs") version "5.0.14"
911
id("de.thetaphi.forbiddenapis") version "3.8"
10-
11-
id("tracer-version")
1212
id("io.github.gradle-nexus.publish-plugin") version "2.0.0"
13-
1413
id("com.gradleup.shadow") version "8.3.6" apply false
1514
id("me.champeau.jmh") version "0.7.3" apply false
1615
id("org.gradle.playframework") version "0.13" apply false

‎buildSrc/build.gradle.kts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,13 @@ gradlePlugin {
2727
implementationClass = "datadog.gradle.plugin.CallSiteInstrumentationPlugin"
2828
}
2929
create("tracer-version-plugin") {
30-
id = "tracer-version"
30+
id = "datadog.tracer-version"
3131
implementationClass = "datadog.gradle.plugin.version.TracerVersionPlugin"
3232
}
33+
create("dump-hanged-test-plugin") {
34+
id = "datadog.dump-hanged-test"
35+
implementationClass = "datadog.gradle.plugin.dump.DumpHangedTestPlugin"
36+
}
3337
create("supported-config-generation") {
3438
id = "supported-config-generator"
3539
implementationClass = "datadog.gradle.plugin.config.SupportedConfigPlugin"

‎buildSrc/src/integTest/kotlin/datadog/gradle/plugin/version/DumpHangedTestIntegrationTest.kt‎

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
package datadog.gradle.plugin.version
2+
3+
import org.gradle.testkit.runner.GradleRunner
4+
import org.gradle.testkit.runner.UnexpectedBuildFailure
5+
import org.junit.jupiter.api.Assertions.assertFalse
6+
import org.junit.jupiter.api.Assertions.assertTrue
7+
import org.junit.jupiter.api.Test
8+
import org.junit.jupiter.api.assertNotNull
9+
import org.junit.jupiter.api.io.TempDir
10+
import java.io.File
11+
import java.nio.file.Paths
12+
13+
class DumpHangedTestIntegrationTest {
14+
@Test
15+
fun `should not take dumps`(@TempDir projectDir: File) {
16+
val output = runGradleTest(projectDir, testSleep = 1000)
17+
18+
// Assert Gradle output has no evidence of taking dumps.
19+
assertFalse(output.contains("Taking dumps after 15 seconds delay for :test"))
20+
assertFalse(output.contains("Requesting stop of task ':test' as it has exceeded its configured timeout of 20s."))
21+
22+
assertTrue(file(projectDir, "build").exists()) // Assert build happened.
23+
assertFalse(file(projectDir, "build", "dumps").exists()) // Assert no dumps created.
24+
}
25+
26+
@Test
27+
fun `should take dumps`(@TempDir projectDir: File) {
28+
val output = runGradleTest(projectDir, testSleep = 25_0000)
29+
30+
// Assert Gradle output has evidence of taking dumps.
31+
assertTrue(output.contains("Taking dumps after 15 seconds delay for :test"))
32+
assertTrue(output.contains("Requesting stop of task ':test' as it has exceeded its configured timeout of 20s."))
33+
34+
assertTrue(file(projectDir, "build").exists()) // Assert build happened.
35+
36+
val dumps = file(projectDir, "build", "dumps")
37+
assertTrue(dumps.exists()) // Assert dumps created.
38+
39+
// Assert actual dumps created.
40+
val dumpFiles = dumps.list()
41+
assertNotNull(dumpFiles.find { it.endsWith(".hprof") })
42+
assertNotNull(dumpFiles.find { it.startsWith("all-thread-dumps") })
43+
}
44+
45+
private fun runGradleTest(projectDir: File, testSleep: Long): List<String> {
46+
file(projectDir, "settings.gradle.kts").writeText(
47+
"""
48+
rootProject.name = "test-project"
49+
""".trimIndent()
50+
)
51+
52+
file(projectDir, "build.gradle.kts").writeText(
53+
"""
54+
import java.time.Duration
55+
56+
plugins {
57+
id("java")
58+
id("datadog.dump-hanged-test")
59+
}
60+
61+
group = "datadog.dump.test"
62+
63+
repositories {
64+
mavenCentral()
65+
}
66+
67+
dependencies {
68+
testImplementation(platform("org.junit:junit-bom:5.10.0"))
69+
testImplementation("org.junit.jupiter:junit-jupiter")
70+
testRuntimeOnly("org.junit.platform:junit-platform-launcher")
71+
}
72+
73+
dumpHangedTest {
74+
// Set the dump offset for 5 seconds to trigger taking dumps after 15 seconds.
75+
dumpOffset.set(5)
76+
}
77+
78+
tasks.withType<Test>().configureEach {
79+
// Set test timeout after 20 seconds.
80+
timeout.set(Duration.ofSeconds(20))
81+
82+
useJUnitPlatform()
83+
}
84+
""".trimIndent()
85+
)
86+
87+
file(projectDir, "src", "test", "java", "SimpleTest.java", makeDirectory = true).writeText(
88+
"""
89+
import org.junit.jupiter.api.Test;
90+
91+
public class SimpleTest {
92+
@Test
93+
public void test() throws InterruptedException {
94+
Thread.sleep($testSleep);
95+
}
96+
}
97+
""".trimIndent()
98+
)
99+
100+
try {
101+
val buildResult = GradleRunner.create()
102+
.forwardOutput()
103+
.withPluginClasspath()
104+
.withArguments("test")
105+
.withProjectDir(projectDir)
106+
.build()
107+
108+
return buildResult.output.lines()
109+
} catch (e: UnexpectedBuildFailure) {
110+
return e.buildResult.output.lines()
111+
}
112+
}
113+
114+
private fun file(projectDir: File, vararg parts: String, makeDirectory: Boolean = false): File {
115+
val f = Paths.get(projectDir.absolutePath, *parts).toFile()
116+
117+
if (makeDirectory) {
118+
f.parentFile.mkdirs()
119+
}
120+
121+
return f
122+
}
123+
}

‎buildSrc/src/integTest/kotlin/datadog/gradle/plugin/version/TracerVersionIntegrationTest.kt‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -265,7 +265,7 @@ class TracerVersionIntegrationTest {
265265
File(projectDir, "build.gradle.kts").writeText(
266266
"""
267267
plugins {
268-
id("tracer-version")
268+
id("datadog.tracer-version")
269269
}
270270
271271
tasks.register("printVersion") {

‎buildSrc/src/main/kotlin/datadog/gradle/plugin/dump/DumpHangedTestPlugin.kt‎

Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
package datadog.gradle.plugin.dump
2+
3+
import org.gradle.api.Plugin
4+
import org.gradle.api.Project
5+
import org.gradle.api.Task
6+
import org.gradle.api.model.ObjectFactory
7+
import org.gradle.api.provider.Property
8+
import org.gradle.api.provider.Provider
9+
import org.gradle.api.services.BuildService
10+
import org.gradle.api.services.BuildServiceParameters
11+
import org.gradle.api.tasks.testing.Test
12+
import org.gradle.kotlin.dsl.extra
13+
import org.gradle.kotlin.dsl.withType
14+
import java.io.File
15+
import java.io.IOException
16+
import java.lang.ProcessBuilder.Redirect
17+
import java.time.Duration
18+
import java.util.concurrent.Executors
19+
import java.util.concurrent.ScheduledExecutorService
20+
import java.util.concurrent.ScheduledFuture
21+
import java.util.concurrent.TimeUnit
22+
import javax.inject.Inject
23+
24+
/**
25+
* Plugin to collect thread and heap dumps for hanged tests.
26+
*/
27+
class DumpHangedTestPlugin : Plugin<Project> {
28+
companion object {
29+
private const val DUMP_FUTURE_KEY = "dumping_future"
30+
}
31+
32+
/** Plugin properties */
33+
abstract class DumpHangedTestProperties @Inject constructor(objects: ObjectFactory) {
34+
// Time offset (in seconds) before a test reaches its timeout at which dumps should be started.
35+
// Defaults to 60 seconds.
36+
val dumpOffset: Property<Long> = objects.property(Long::class.java)
37+
}
38+
39+
/** Executor wrapped with proper Gradle lifecycle. */
40+
abstract class DumpSchedulerService : BuildService<BuildServiceParameters.None>, AutoCloseable {
41+
private val executor: ScheduledExecutorService =
42+
Executors.newSingleThreadScheduledExecutor { r -> Thread(r, "hanged-test-dump").apply { isDaemon = true } }
43+
44+
fun schedule(task: () -> Unit, delay: Duration): ScheduledFuture<*> =
45+
executor.schedule(task, delay.toMillis(), TimeUnit.MILLISECONDS)
46+
47+
override fun close() {
48+
executor.shutdownNow()
49+
}
50+
}
51+
52+
override fun apply(project: Project) {
53+
if (project.rootProject != project) {
54+
return
55+
}
56+
57+
val scheduler = project.gradle.sharedServices
58+
.registerIfAbsent("dumpHangedTestScheduler", DumpSchedulerService::class.java)
59+
60+
// Create plugin properties.
61+
val props = project.extensions.create("dumpHangedTest", DumpHangedTestProperties::class.java)
62+
63+
fun configure(p: Project) {
64+
p.tasks.withType<Test>().configureEach {
65+
doFirst { schedule(this, scheduler, props) }
66+
doLast { cleanup(this) }
67+
}
68+
}
69+
70+
configure(project)
71+
72+
project.subprojects(::configure)
73+
}
74+
75+
private fun schedule(t: Task, scheduler: Provider<DumpSchedulerService>, props: DumpHangedTestProperties) {
76+
val taskName = t.path
77+
78+
if (t.extra.has(DUMP_FUTURE_KEY)) {
79+
t.logger.info("Taking dumps already scheduled for $taskName")
80+
return
81+
}
82+
83+
val dumpOffset = props.dumpOffset.getOrElse(60)
84+
val delay = t.timeout.map { it.minusSeconds(dumpOffset) }.orNull
85+
86+
if (delay == null || delay.seconds < 0) {
87+
t.logger.info("Taking dumps has invalid timeout configured for $taskName")
88+
return
89+
}
90+
91+
val future = scheduler.get().schedule({
92+
t.logger.quiet("Taking dumps after ${delay.seconds} seconds delay for $taskName")
93+
94+
takeDump(t)
95+
}, delay)
96+
97+
t.extra.set(DUMP_FUTURE_KEY, future)
98+
}
99+
100+
private fun takeDump(t: Task) {
101+
try {
102+
// Use Gradle's build dir and adjust for CI artifacts collection if needed.
103+
val dumpsDir: File = t.project.layout.buildDirectory
104+
.dir("dumps")
105+
.map { dir ->
106+
if (t.project.providers.environmentVariable("CI").isPresent) {
107+
// Move reports into the folder collected by the collect_reports.sh script.
108+
File(
109+
dir.asFile.absolutePath.replace(
110+
"dd-trace-java/dd-java-agent",
111+
"dd-trace-java/workspace/dd-java-agent"
112+
)
113+
)
114+
} else {
115+
dir.asFile
116+
}
117+
}
118+
.get()
119+
120+
dumpsDir.mkdirs()
121+
122+
fun file(name: String, ext: String = "log") =
123+
File(dumpsDir, "$name-${System.currentTimeMillis()}.$ext")
124+
125+
// For simplicity, use `0` as the PID, which collects all thread dumps across JVMs.
126+
val allThreadsFile = file("all-thread-dumps")
127+
runCmd(Redirect.to(allThreadsFile), "jcmd", "0", "Thread.print", "-l")
128+
129+
// Collect all JVMs pids.
130+
val allJavaProcessesFile = file("all-java-processes")
131+
runCmd(Redirect.to(allJavaProcessesFile), "jcmd", "-l")
132+
133+
// Collect pids for 'Gradle Test Executor'.
134+
val pids = allJavaProcessesFile.readLines()
135+
.filter { it.contains("Gradle Test Executor") }
136+
.map { it.substringBefore(' ') }
137+
138+
pids.forEach { pid ->
139+
// Collect heap dump by pid.
140+
val heapDumpPath = file("${pid}-heap-dump", "hprof").absolutePath
141+
runCmd(Redirect.INHERIT, "jcmd", pid, "GC.heap_dump", heapDumpPath)
142+
143+
// Collect thread dump by pid.
144+
val threadDumpFile = file("${pid}-thread-dump")
145+
runCmd(Redirect.to(threadDumpFile), "jcmd", pid, "Thread.print", "-l")
146+
}
147+
} catch (e: Throwable) {
148+
t.logger.warn("Taking dumps failed with error: ${e.message}, for ${t.path}")
149+
}
150+
}
151+
152+
private fun cleanup(t: Task) {
153+
val future = t.extra
154+
.takeIf { it.has(DUMP_FUTURE_KEY) }
155+
?.get(DUMP_FUTURE_KEY) as? ScheduledFuture<*>
156+
157+
if (future != null && !future.isDone) {
158+
t.logger.info("Taking dump canceled with remaining delay of ${future.getDelay(TimeUnit.SECONDS)} seconds for ${t.path}")
159+
future.cancel(false)
160+
}
161+
}
162+
163+
private fun runCmd(
164+
redirectTo: Redirect,
165+
vararg args: String
166+
) {
167+
val exitCode = ProcessBuilder(*args)
168+
.redirectErrorStream(true)
169+
.redirectOutput(redirectTo)
170+
.start()
171+
.waitFor()
172+
173+
if (exitCode != 0) {
174+
throw IOException("Process failed: ${args.joinToString(" ")}, exit code: $exitCode")
175+
}
176+
}
177+
}

‎gradle/configure_tests.gradle‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
import java.time.Duration
22
import java.time.temporal.ChronoUnit
33

4-
apply from: "$rootDir/gradle/dump_hanging_test.gradle"
5-
64
def isTestingInstrumentation(Project project) {
75
return [
86
"junit-4.10",

0 commit comments

Comments
 (0)