From 27e96308ed69910cd6e19575591ca828918302ac Mon Sep 17 00:00:00 2001 From: Sebastien Tardif Date: Thu, 16 Jul 2026 09:13:41 -0400 Subject: [PATCH 1/2] Improve Docker Compose ProcessRunner's handling of failure cases 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 See gh-50963 --- .../docker/compose/core/ProcessRunner.java | 5 +- .../compose/core/ProcessRunnerTests.java | 100 ++++++++++++++---- 2 files changed, 81 insertions(+), 24 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 b0491330f59..6a6694f3daf 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 @@ -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 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 5fc96511bd3..b29af2d392a 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,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 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(); + }); + } + } } From bf8731693b28f70cc5bd91fca4420337bc936807 Mon Sep 17 00:00:00 2001 From: Andy Wilkinson Date: Wed, 2 Sep 2026 09:03:14 +0100 Subject: [PATCH 2/2] Polish "Improve Docker Compose ProcessRunner's handling of failure cases" See gh-50963 Signed-off-by: Andy Wilkinson --- .../boot/docker/compose/core/ProcessRunnerTests.java | 8 +++----- 1 file changed, 3 insertions(+), 5 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 b29af2d392a..370bb88551c 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 @@ -17,10 +17,12 @@ package org.springframework.boot.docker.compose.core; import java.nio.file.Files; +import java.nio.file.LinkOption; import java.nio.file.Path; import java.time.Duration; import java.util.concurrent.atomic.AtomicReference; +import org.awaitility.Awaitility; import org.junit.jupiter.api.Nested; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.condition.DisabledOnOs; @@ -75,11 +77,7 @@ class ProcessRunnerTests { } }, "process-runner-interrupt-test"); runner.start(); - long deadline = System.currentTimeMillis() + 5000; - while (!Files.exists(pidFile) && System.currentTimeMillis() < deadline) { - Thread.sleep(50); - } - assertThat(pidFile).exists(); + Awaitility.await().until(() -> Files.exists(pidFile, LinkOption.NOFOLLOW_LINKS)); long pid = Long.parseLong(Files.readString(pidFile).trim()); assertThat(ProcessHandle.of(pid)).isPresent().get().matches(ProcessHandle::isAlive); runner.interrupt();