spring/spring-boot
1
0
Fork 0
mirror of https://github.com/spring-projects/spring-boot.git synced 2026-09-17 12:09:16 +00:00
Code Issues Projects Releases Wiki Activity

Improve Docker Compose ProcessRunner's handling of failure cases

Browse Source 
Release the output reader latch in a finally block so a failing stream
read or output consumer cannot leave ProcessRunner.run blocked on
CountDownLatch.await after the child process has exited.

Destroy the child process when waitFor is interrupted so Docker Compose
commands are not left running as orphans.

Signed-off-by: Sebastien Tardif <sebtardif@ncf.ca>

See gh-50963
This commit is contained in:
Sebastien Tardif
2026-09-02 09:02:00 +01:00
committed by Andy Wilkinson
parent 837f44f764
commit 27e96308ed
2 changed files with 81 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files
core/spring-boot-docker-compose/src/main/java/org/springframework/boot/docker/compose/core/ProcessRunner.java
+4 -1
View File
@@ -124,6 +124,7 @@ class ProcessRunner {
}
catch (InterruptedException ex) {
Thread.currentThread().interrupt();
process.destroy();
throw new IllegalStateException("Interrupted waiting for %s".formatted(process), ex);
}
}
@@ -162,11 +163,13 @@ class ProcessRunner {
}
line = reader.readLine();
}
this.latch.countDown();
}
catch (IOException ex) {
throw new UncheckedIOException("Failed to read process stream", ex);
}
finally {
this.latch.countDown();
}
}
@Override
core/spring-boot-docker-compose/src/test/java/org/springframework/boot/docker/compose/core/ProcessRunnerTests.java
+77 -23
View File
@@ -16,7 +16,15 @@
package org.springframework.boot.docker.compose.core;
import java.nio.file.Files;
import java.nio.file.Path;
import java.time.Duration;
import java.util.concurrent.atomic.AtomicReference;
import org.junit.jupiter.api.Nested;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.condition.DisabledOnOs;
import org.junit.jupiter.api.condition.OS;
import org.springframework.boot.testsupport.process.DisabledIfProcessUnavailable;
@@ -29,24 +37,11 @@ import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
* @author Moritz Halbritter
* @author Andy Wilkinson
* @author Phillip Webb
* @author Sebastien Tardif
*/
@DisabledIfProcessUnavailable("docker")
class ProcessRunnerTests {
private ProcessRunner processRunner = new ProcessRunner();
@Test
void run() {
String out = this.processRunner.run("docker", "--version");
assertThat(out).isNotEmpty();
}
@Test
void runWhenHasOutputConsumer() {
StringBuilder output = new StringBuilder();
this.processRunner.run(output::append, "docker", "--version");
assertThat(output.toString()).isNotEmpty();
}
private final ProcessRunner processRunner = new ProcessRunner();
@Test
void runWhenProcessDoesNotStart() {
@@ -55,14 +50,73 @@ class ProcessRunnerTests {
}
@Test
void runWhenProcessReturnsNonZeroExitCode() {
assertThatExceptionOfType(ProcessExitException.class)
.isThrownBy(() -> this.processRunner.run("docker", "-thisdoesntwork"))
.satisfies((ex) -> {
assertThat(ex.getExitCode()).isGreaterThan(0);
assertThat(ex.getStdOut()).isEmpty();
assertThat(ex.getStdErr()).isNotEmpty();
});
@DisabledOnOs(OS.WINDOWS)
void runWhenOutputConsumerThrowsDoesNotHang() throws InterruptedException {
Thread runner = new Thread(() -> this.processRunner.run((line) -> {
throw new IllegalStateException("boom");
}, "echo", "hello"), "process-runner-consumer-throw-test");
runner.start();
runner.join(Duration.ofSeconds(5).toMillis());
assertThat(runner.isAlive()).isFalse();
}
@Test
@DisabledOnOs(OS.WINDOWS)
void runWhenInterruptedDestroysChildProcess() throws Exception {
Path pidFile = Files.createTempFile("process-runner-", ".pid");
Files.delete(pidFile);
AtomicReference<Throwable> error = new AtomicReference<>();
Thread runner = new Thread(() -> {
try {
this.processRunner.run("sh", "-c", "echo $$ > '" + pidFile + "'; exec sleep 60");
}
catch (Throwable ex) {
error.set(ex);
}
}, "process-runner-interrupt-test");
runner.start();
long deadline = System.currentTimeMillis() + 5000;
while (!Files.exists(pidFile) && System.currentTimeMillis() < deadline) {
Thread.sleep(50);
}
assertThat(pidFile).exists();
long pid = Long.parseLong(Files.readString(pidFile).trim());
assertThat(ProcessHandle.of(pid)).isPresent().get().matches(ProcessHandle::isAlive);
runner.interrupt();
runner.join(Duration.ofSeconds(5).toMillis());
assertThat(runner.isAlive()).isFalse();
assertThat(error.get()).isInstanceOf(IllegalStateException.class);
assertThat(ProcessHandle.of(pid).map(ProcessHandle::isAlive).orElse(false)).isFalse();
}
@Nested
@DisabledIfProcessUnavailable("docker")
class WhenDockerIsAvailable {
@Test
void run() {
String out = ProcessRunnerTests.this.processRunner.run("docker", "--version");
assertThat(out).isNotEmpty();
}
@Test
void runWhenHasOutputConsumer() {
StringBuilder output = new StringBuilder();
ProcessRunnerTests.this.processRunner.run(output::append, "docker", "--version");
assertThat(output.toString()).isNotEmpty();
}
@Test
void runWhenProcessReturnsNonZeroExitCode() {
assertThatExceptionOfType(ProcessExitException.class)
.isThrownBy(() -> ProcessRunnerTests.this.processRunner.run("docker", "-thisdoesntwork"))
.satisfies((ex) -> {
assertThat(ex.getExitCode()).isGreaterThan(0);
assertThat(ex.getStdOut()).isEmpty();
assertThat(ex.getStdErr()).isNotEmpty();
});
}
}
}