Docker Compose ProcessRunner can hang or leave orphan processes

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

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<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();
+	}
+
 }