Skip to content

Commit e5e0164

Browse files
Refactor ProcessManager.cleanupSpec() to remove infinite loop with deterministic control flow. (#10747)
Refactor cleanupSpec() to remove infinite loop with deterministic control flow. Added missing import Merge branch 'master' into alexeyk/fixed-infinite-loop Fixed review comments. Co-authored-by: alexey.kuznetsov <alexey.kuznetsov@datadoghq.com>
1 parent 6350798 commit e5e0164

File tree

2 files changed

+34
-20
lines changed

2 files changed

+34
-20
lines changed

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ out/
6060

6161
# Others #
6262
##########
63+
/dumps
6364
/logs/*
6465
/bin
6566
/out

dd-smoke-tests/src/main/groovy/datadog/smoketest/ProcessManager.groovy

Lines changed: 33 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ package datadog.smoketest
33
import datadog.trace.agent.test.utils.PortUtils
44
import java.nio.file.Files
55
import java.nio.file.Paths
6-
import java.util.concurrent.TimeoutException
6+
import java.util.concurrent.TimeUnit
77
import spock.lang.AutoCleanup
88
import spock.lang.Shared
99
import spock.lang.Specification
@@ -108,32 +108,45 @@ abstract class ProcessManager extends Specification {
108108
}
109109

110110
def cleanupSpec() {
111+
Throwable firstFailure = null
111112
testedProcesses.each { tp ->
112-
int maxAttempts = 10
113+
if (tp == null) {
114+
return // closure continue — skip null slots
115+
}
116+
113117
Integer exitValue
114-
for (int attempt = 1; attempt <= maxAttempts != null; attempt++) {
115-
try {
116-
exitValue = tp?.exitValue()
117-
break
118+
try {
119+
exitValue = tp.exitValue()
120+
} catch (Throwable ignored) {
121+
System.err.println("Destroying instrumented process")
122+
tp.destroy()
123+
124+
if (!tp.waitFor(5, TimeUnit.SECONDS)) {
125+
System.err.println("Destroying instrumented process (forced)")
126+
tp.destroyForcibly()
127+
tp.waitFor(10, TimeUnit.SECONDS)
118128
}
119-
catch (Throwable ignored) {
120-
if (attempt == 1) {
121-
System.out.println("Destroying instrumented process")
122-
tp.destroy()
123-
}
124-
if (attempt == maxAttempts - 1) {
125-
System.out.println("Destroying instrumented process (forced)")
126-
tp.destroyForcibly()
129+
130+
try {
131+
exitValue = tp.exitValue()
132+
} catch (Throwable ignoredAgain) {
133+
// Process did not exit even after SIGKILL — record failure but continue
134+
// cleaning up any remaining processes before propagating.
135+
def failure = new RuntimeException("Instrumented process failed to exit after SIGKILL")
136+
if (firstFailure == null) {
137+
firstFailure = failure
138+
} else {
139+
firstFailure.addSuppressed(failure)
127140
}
128-
sleep 1_000
141+
return // closure continue
129142
}
130143
}
131144

132-
if (exitValue != null) {
133-
System.out.println("Instrumented process exited with " + exitValue)
134-
} else if (tp != null) {
135-
throw new TimeoutException("Instrumented process failed to exit")
136-
}
145+
System.err.println("Instrumented process exited with " + exitValue)
146+
}
147+
148+
if (firstFailure != null) {
149+
throw firstFailure
137150
}
138151
}
139152

0 commit comments

Comments
 (0)