From 30d42d7bbffab60ac2f882079f1231e94adfbae6 Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Thu, 16 Jul 2026 09:13:41 -0400 Subject: [PATCH 1/2] Docker Compose ProcessRunner can hang or leave orphan processes 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 --- .../docker/compose/core/ProcessRunner.java | 5 +- .../compose/core/ProcessRunnerTests.java | 55 ++++++++++++++++++- 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/core/spring-boot-docker-compose/src/main/java/org/springframework/boot/docker/compose/core/ProcessRunner.java b/core/spring-boot-docker-compose/src/main/java/org/springframework/boot/docker/compose/core/ProcessRunner.java index 2aaac547822a..6fd72d1c83a9 100644 --- a/core/spring-boot-docker-compose/src/main/java/org/springframework/boot/docker/compose/core/ProcessRunner.java +++ b/core/spring-boot-docker-compose/src/main/java/org/springframework/boot/docker/compose/core/ProcessRunner.java @@ -139,6 +139,7 @@ private int waitForProcess(Process process) { } catch (InterruptedException ex) { Thread.currentThread().interrupt(); + process.destroy(); throw new IllegalStateException("Interrupted waiting for %s".formatted(process), ex); } } @@ -177,11 +178,13 @@ public void run() { } line = reader.readLine(); } - this.latch.countDown(); } catch (IOException ex) { throw new UncheckedIOException("Failed to read process stream", ex); } + finally { + this.latch.countDown(); + } } @Override diff --git a/core/spring-boot-docker-compose/src/test/java/org/springframework/boot/docker/compose/core/ProcessRunnerTests.java b/core/spring-boot-docker-compose/src/test/java/org/springframework/boot/docker/compose/core/ProcessRunnerTests.java index 5fc96511bd39..cf501fecbf89 100644 --- a/core/spring-boot-docker-compose/src/test/java/org/springframework/boot/docker/compose/core/ProcessRunnerTests.java +++ b/core/spring-boot-docker-compose/src/test/java/org/springframework/boot/docker/compose/core/ProcessRunnerTests.java @@ -16,7 +16,16 @@ 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.Test; +import org.junit.jupiter.api.Timeout; +import org.junit.jupiter.api.Timeout.ThreadMode; +import org.junit.jupiter.api.condition.DisabledOnOs; +import org.junit.jupiter.api.condition.OS; import org.springframework.boot.testsupport.process.DisabledIfProcessUnavailable; @@ -29,19 +38,21 @@ * @author Moritz Halbritter * @author Andy Wilkinson * @author Phillip Webb + * @author Sebastien Tardif */ -@DisabledIfProcessUnavailable("docker") class ProcessRunnerTests { - private ProcessRunner processRunner = new ProcessRunner(); + private final ProcessRunner processRunner = new ProcessRunner(); @Test + @DisabledIfProcessUnavailable("docker") void run() { String out = this.processRunner.run("docker", "--version"); assertThat(out).isNotEmpty(); } @Test + @DisabledIfProcessUnavailable("docker") void runWhenHasOutputConsumer() { StringBuilder output = new StringBuilder(); this.processRunner.run(output::append, "docker", "--version"); @@ -55,6 +66,7 @@ void runWhenProcessDoesNotStart() { } @Test + @DisabledIfProcessUnavailable("docker") void runWhenProcessReturnsNonZeroExitCode() { assertThatExceptionOfType(ProcessExitException.class) .isThrownBy(() -> this.processRunner.run("docker", "-thisdoesntwork")) @@ -65,4 +77,43 @@ void runWhenProcessReturnsNonZeroExitCode() { }); } + @Test + @DisabledOnOs(OS.WINDOWS) + @Timeout(value = 5, threadMode = ThreadMode.SEPARATE_THREAD) + void runWhenOutputConsumerThrowsDoesNotHang() { + this.processRunner.run((line) -> { + throw new IllegalStateException("boom"); + }, "echo", "hello"); + } + + @Test + @DisabledOnOs(OS.WINDOWS) + @Timeout(value = 10, threadMode = ThreadMode.SEPARATE_THREAD) + void runWhenInterruptedDestroysChildProcess() throws Exception { + Path pidFile = Files.createTempFile("process-runner-", ".pid"); + Files.delete(pidFile); + AtomicReference 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(); + } + } From 70904f79f282b611bc33e5c6c03153fd6f692320 Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Wed, 26 Aug 2026 06:40:31 -0700 Subject: [PATCH 2/2] Polish ProcessRunnerTests after review Move docker-backed cases into a nested class and drop @Timeout in favor of a bounded join on the hang regression. Signed-off-by: Sebastien Tardif --- .../compose/core/ProcessRunnerTests.java | 71 ++++++++++--------- 1 file changed, 37 insertions(+), 34 deletions(-) diff --git a/core/spring-boot-docker-compose/src/test/java/org/springframework/boot/docker/compose/core/ProcessRunnerTests.java b/core/spring-boot-docker-compose/src/test/java/org/springframework/boot/docker/compose/core/ProcessRunnerTests.java index cf501fecbf89..b29af2d392a5 100644 --- a/core/spring-boot-docker-compose/src/test/java/org/springframework/boot/docker/compose/core/ProcessRunnerTests.java +++ b/core/spring-boot-docker-compose/src/test/java/org/springframework/boot/docker/compose/core/ProcessRunnerTests.java @@ -21,9 +21,8 @@ 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.Timeout; -import org.junit.jupiter.api.Timeout.ThreadMode; import org.junit.jupiter.api.condition.DisabledOnOs; import org.junit.jupiter.api.condition.OS; @@ -44,51 +43,25 @@ class ProcessRunnerTests { private final ProcessRunner processRunner = new ProcessRunner(); - @Test - @DisabledIfProcessUnavailable("docker") - void run() { - String out = this.processRunner.run("docker", "--version"); - assertThat(out).isNotEmpty(); - } - - @Test - @DisabledIfProcessUnavailable("docker") - void runWhenHasOutputConsumer() { - StringBuilder output = new StringBuilder(); - this.processRunner.run(output::append, "docker", "--version"); - assertThat(output.toString()).isNotEmpty(); - } - @Test void runWhenProcessDoesNotStart() { assertThatExceptionOfType(ProcessStartException.class) .isThrownBy(() -> this.processRunner.run("iverymuchdontexist", "--version")); } - @Test - @DisabledIfProcessUnavailable("docker") - 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(); - }); - } - @Test @DisabledOnOs(OS.WINDOWS) - @Timeout(value = 5, threadMode = ThreadMode.SEPARATE_THREAD) - void runWhenOutputConsumerThrowsDoesNotHang() { - this.processRunner.run((line) -> { + void runWhenOutputConsumerThrowsDoesNotHang() throws InterruptedException { + Thread runner = new Thread(() -> this.processRunner.run((line) -> { throw new IllegalStateException("boom"); - }, "echo", "hello"); + }, "echo", "hello"), "process-runner-consumer-throw-test"); + runner.start(); + runner.join(Duration.ofSeconds(5).toMillis()); + assertThat(runner.isAlive()).isFalse(); } @Test @DisabledOnOs(OS.WINDOWS) - @Timeout(value = 10, threadMode = ThreadMode.SEPARATE_THREAD) void runWhenInterruptedDestroysChildProcess() throws Exception { Path pidFile = Files.createTempFile("process-runner-", ".pid"); Files.delete(pidFile); @@ -116,4 +89,34 @@ void runWhenInterruptedDestroysChildProcess() throws Exception { 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(); + }); + } + + } + }