From 512b5a8298b8edd06c46d27afe2dea69e977cfb1 Mon Sep 17 00:00:00 2001 From: Maxwell Elliott Date: Fri, 15 May 2026 05:08:14 -0400 Subject: [PATCH 1/2] Reproducer test for #256: process() hangs when subprocess reads stdin MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds an @Ignore'd JUnit reproducer that demonstrates the most likely underlying cause of #256 (bazel-diff freezes when aspect CLI is installed). `process()` calls `ProcessBuilder.start()` without ever redirecting or closing the subprocess's stdin pipe, so any subprocess that reads from stdin (as the aspect CLI v1's interactive first-run path appears to do) blocks forever waiting for input. The test spawns `cat` (which blocks on stdin read), wraps the call in `withTimeoutOrNull(5_000)`, and asserts the result is null — proving the deadlock. Confirmed locally: with @Ignore removed, the test runs exactly 5s and passes; with @Ignore in place CI stays green. No production code change in this PR. The fix is intentionally out-of-scope; this PR just locks in a regression target for whoever picks up the work on #256. Co-Authored-By: Claude Opus 4.7 (1M context) --- cli/BUILD | 6 ++ .../process/ProcessStdinHangReproducerTest.kt | 65 +++++++++++++++++++ 2 files changed, 71 insertions(+) create mode 100644 cli/src/test/kotlin/com/bazel_diff/process/ProcessStdinHangReproducerTest.kt diff --git a/cli/BUILD b/cli/BUILD index ee701a5..84f5e0e 100644 --- a/cli/BUILD +++ b/cli/BUILD @@ -120,6 +120,12 @@ kt_jvm_test( runtime_deps = [":cli-test-lib"], ) +kt_jvm_test( + name = "ProcessStdinHangReproducerTest", + test_class = "com.bazel_diff.process.ProcessStdinHangReproducerTest", + runtime_deps = [":cli-test-lib"], +) + kt_jvm_test( name = "E2ETest", timeout = "long", diff --git a/cli/src/test/kotlin/com/bazel_diff/process/ProcessStdinHangReproducerTest.kt b/cli/src/test/kotlin/com/bazel_diff/process/ProcessStdinHangReproducerTest.kt new file mode 100644 index 0000000..c645350 --- /dev/null +++ b/cli/src/test/kotlin/com/bazel_diff/process/ProcessStdinHangReproducerTest.kt @@ -0,0 +1,65 @@ +package com.bazel_diff.process + +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.withTimeoutOrNull +import org.junit.Ignore +import org.junit.Test +import kotlin.test.assertNull + +/** + * Reproducer for https://github.com/Tinder/bazel-diff/issues/256 + * ("bug: bazel-diff freezes when aspect CLI is installed"). + * + * The user reported that with `.bazeliskrc` set to download aspect CLI + * (`USE_BAZEL_VERSION=aspect/...`), every `bazel-diff` invocation hangs forever. + * An `strace` on the Java process showed it parked in a `FUTEX_WAIT`, i.e. waiting + * on the bazel subprocess that never makes progress. The cross-posted aspect-cli + * issue (aspect-build/aspect-cli-legacy#41) corroborates this and Alex Eagle + * suggested unsetting `BAZELISK_BASE_URL` / `USE_BAZEL_VERSION` as a workaround. + * + * Likely root cause exercised by this test: [process] starts the subprocess via + * `ProcessBuilder.start()` without redirecting stdin. Java's default for stdin is + * `Redirect.PIPE`, so the subprocess receives an open, never-closed stdin pipe. + * Any subprocess that reads from stdin (e.g., the aspect CLI's interactive + * "first run" path) blocks indefinitely waiting for input that bazel-diff never + * sends, and `process.waitFor()` then blocks forever. + * + * This test reproduces the pattern with a minimal subprocess (`cat`) that simply + * reads from stdin. It is marked `@Ignore` because, by design, the test would + * either deadlock CI or assert-on-a-timeout — neither is desirable in the + * regularly-scheduled test suite. Removing `@Ignore` and watching the + * `withTimeoutOrNull` return non-null is the expected signal that a fix + * (e.g., `redirectInput(Redirect.from(/dev/null))` or + * `process.outputStream.close()` after `start()`) is in place. + * + * The follow-up fix is intentionally out of scope for this PR; see issue #256 + * for the discussion. + */ +@OptIn(ExperimentalCoroutinesApi::class) +class ProcessStdinHangReproducerTest { + + @Ignore("Reproducer for #256 — proves process() deadlocks when the subprocess reads from stdin.") + @Test + fun `process hangs when subprocess reads from stdin and parent never closes it`() = runBlocking { + // `cat` with no args reads from stdin until EOF. Because process() never + // closes the subprocess's stdin pipe, `cat` blocks on read forever, never + // writes anything to stdout, and `process.waitFor()` (and the stdout + // line-flow that precedes it) blocks forever. + val result = + withTimeoutOrNull(timeMillis = 5_000) { + process( + "cat", + stdout = Redirect.CAPTURE, + stderr = Redirect.SILENT, + ) + } + + // `null` here means the timeout fired, i.e. process() hung — the bug is + // reproduced. If/when the underlying behaviour is fixed (stdin is closed + // or redirected to /dev/null), `cat` will see EOF immediately, exit with + // status 0, and `result` will be a populated `ProcessResult`. Flip this + // assertion (and drop `@Ignore`) once the fix lands. + assertNull(result, "Expected process() to deadlock; if it returned, #256 may be fixed.") + } +} From 9c4ed66b32d84fb9b90838366bf787573806006d Mon Sep 17 00:00:00 2001 From: Maxwell Elliott Date: Fri, 15 May 2026 10:21:24 -0400 Subject: [PATCH 2/2] Fix #256: close subprocess stdin so reads see EOF MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `process()` started subprocesses via `ProcessBuilder.start()` without redirecting or closing stdin. Java defaults stdin to `Redirect.PIPE`, so the subprocess received an open, never-fed pipe; any subprocess that read from stdin on startup (e.g. aspect CLI's interactive first-run path) blocked forever on `read()`, and `process.waitFor()` blocked too — matching the user-reported `FUTEX_WAIT` strace. Close `process.outputStream` immediately after `start()` so the subprocess sees EOF on stdin, mirroring what `ExternalRepoResolver.runProcessAndCaptureFirstLine` already does. Flip the @Ignore'd reproducer into a real regression test and rename it to `ProcessStdinHangRegressionTest` to match the codebase's `*RegressionTest` convention. The test now asserts that `cat` exits cleanly within the 5s timeout — without the fix it hangs and the timeout fires. Co-Authored-By: Claude Opus 4.7 (1M context) --- cli/BUILD | 4 +- .../kotlin/com/bazel_diff/process/Process.kt | 6 ++ .../process/ProcessStdinHangRegressionTest.kt | 43 ++++++++++++ .../process/ProcessStdinHangReproducerTest.kt | 65 ------------------- 4 files changed, 51 insertions(+), 67 deletions(-) create mode 100644 cli/src/test/kotlin/com/bazel_diff/process/ProcessStdinHangRegressionTest.kt delete mode 100644 cli/src/test/kotlin/com/bazel_diff/process/ProcessStdinHangReproducerTest.kt diff --git a/cli/BUILD b/cli/BUILD index 84f5e0e..ec0c770 100644 --- a/cli/BUILD +++ b/cli/BUILD @@ -121,8 +121,8 @@ kt_jvm_test( ) kt_jvm_test( - name = "ProcessStdinHangReproducerTest", - test_class = "com.bazel_diff.process.ProcessStdinHangReproducerTest", + name = "ProcessStdinHangRegressionTest", + test_class = "com.bazel_diff.process.ProcessStdinHangRegressionTest", runtime_deps = [":cli-test-lib"], ) diff --git a/cli/src/main/kotlin/com/bazel_diff/process/Process.kt b/cli/src/main/kotlin/com/bazel_diff/process/Process.kt index 499e1e4..26c2866 100644 --- a/cli/src/main/kotlin/com/bazel_diff/process/Process.kt +++ b/cli/src/main/kotlin/com/bazel_diff/process/Process.kt @@ -57,6 +57,12 @@ suspend fun process( } .start() + // Close the subprocess's stdin so reads from it see EOF immediately. + // Without this, subprocesses that read stdin on startup (e.g. aspect CLI's + // interactive first-run path, #256) block forever waiting for input we + // never send, and waitFor() then blocks forever too. + process.outputStream.close() + // Handles async consumptions before the blocking output handling. if (stdout is Redirect.Consume) { process.inputStream.lineFlow(stdout.consumer) diff --git a/cli/src/test/kotlin/com/bazel_diff/process/ProcessStdinHangRegressionTest.kt b/cli/src/test/kotlin/com/bazel_diff/process/ProcessStdinHangRegressionTest.kt new file mode 100644 index 0000000..9835881 --- /dev/null +++ b/cli/src/test/kotlin/com/bazel_diff/process/ProcessStdinHangRegressionTest.kt @@ -0,0 +1,43 @@ +package com.bazel_diff.process + +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.withTimeoutOrNull +import org.junit.Test +import kotlin.test.assertEquals +import kotlin.test.assertNotNull + +/** + * Regression test for https://github.com/Tinder/bazel-diff/issues/256 + * ("bug: bazel-diff freezes when aspect CLI is installed"). + * + * Before the fix, [process] started subprocesses via `ProcessBuilder.start()` + * without redirecting or closing stdin. Java defaults stdin to `Redirect.PIPE`, + * so the subprocess received an open, never-closed stdin pipe. Any subprocess + * that reads from stdin (the aspect CLI's interactive first-run path, in #256) + * blocked indefinitely on `read()`, and `process.waitFor()` then blocked too — + * matching the `FUTEX_WAIT` strace the original reporter captured. + * + * The fix closes `process.outputStream` immediately after `start()` so the + * subprocess sees EOF on stdin and exits. + */ +@OptIn(ExperimentalCoroutinesApi::class) +class ProcessStdinHangRegressionTest { + + @Test + fun `process does not hang when subprocess reads from stdin`() = runBlocking { + // `cat` with no args reads from stdin until EOF. Before the fix this hung + // forever; after the fix `cat` sees EOF immediately and exits 0. + val result = + withTimeoutOrNull(timeMillis = 5_000) { + process( + "cat", + stdout = Redirect.CAPTURE, + stderr = Redirect.SILENT, + ) + } + + assertNotNull(result, "process() deadlocked — subprocess stdin was not closed (regression of #256).") + assertEquals(0, result.resultCode) + } +} diff --git a/cli/src/test/kotlin/com/bazel_diff/process/ProcessStdinHangReproducerTest.kt b/cli/src/test/kotlin/com/bazel_diff/process/ProcessStdinHangReproducerTest.kt deleted file mode 100644 index c645350..0000000 --- a/cli/src/test/kotlin/com/bazel_diff/process/ProcessStdinHangReproducerTest.kt +++ /dev/null @@ -1,65 +0,0 @@ -package com.bazel_diff.process - -import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.runBlocking -import kotlinx.coroutines.withTimeoutOrNull -import org.junit.Ignore -import org.junit.Test -import kotlin.test.assertNull - -/** - * Reproducer for https://github.com/Tinder/bazel-diff/issues/256 - * ("bug: bazel-diff freezes when aspect CLI is installed"). - * - * The user reported that with `.bazeliskrc` set to download aspect CLI - * (`USE_BAZEL_VERSION=aspect/...`), every `bazel-diff` invocation hangs forever. - * An `strace` on the Java process showed it parked in a `FUTEX_WAIT`, i.e. waiting - * on the bazel subprocess that never makes progress. The cross-posted aspect-cli - * issue (aspect-build/aspect-cli-legacy#41) corroborates this and Alex Eagle - * suggested unsetting `BAZELISK_BASE_URL` / `USE_BAZEL_VERSION` as a workaround. - * - * Likely root cause exercised by this test: [process] starts the subprocess via - * `ProcessBuilder.start()` without redirecting stdin. Java's default for stdin is - * `Redirect.PIPE`, so the subprocess receives an open, never-closed stdin pipe. - * Any subprocess that reads from stdin (e.g., the aspect CLI's interactive - * "first run" path) blocks indefinitely waiting for input that bazel-diff never - * sends, and `process.waitFor()` then blocks forever. - * - * This test reproduces the pattern with a minimal subprocess (`cat`) that simply - * reads from stdin. It is marked `@Ignore` because, by design, the test would - * either deadlock CI or assert-on-a-timeout — neither is desirable in the - * regularly-scheduled test suite. Removing `@Ignore` and watching the - * `withTimeoutOrNull` return non-null is the expected signal that a fix - * (e.g., `redirectInput(Redirect.from(/dev/null))` or - * `process.outputStream.close()` after `start()`) is in place. - * - * The follow-up fix is intentionally out of scope for this PR; see issue #256 - * for the discussion. - */ -@OptIn(ExperimentalCoroutinesApi::class) -class ProcessStdinHangReproducerTest { - - @Ignore("Reproducer for #256 — proves process() deadlocks when the subprocess reads from stdin.") - @Test - fun `process hangs when subprocess reads from stdin and parent never closes it`() = runBlocking { - // `cat` with no args reads from stdin until EOF. Because process() never - // closes the subprocess's stdin pipe, `cat` blocks on read forever, never - // writes anything to stdout, and `process.waitFor()` (and the stdout - // line-flow that precedes it) blocks forever. - val result = - withTimeoutOrNull(timeMillis = 5_000) { - process( - "cat", - stdout = Redirect.CAPTURE, - stderr = Redirect.SILENT, - ) - } - - // `null` here means the timeout fired, i.e. process() hung — the bug is - // reproduced. If/when the underlying behaviour is fixed (stdin is closed - // or redirected to /dev/null), `cat` will see EOF immediately, exit with - // status 0, and `result` will be a populated `ProcessResult`. Flip this - // assertion (and drop `@Ignore`) once the fix lands. - assertNull(result, "Expected process() to deadlock; if it returned, #256 may be fixed.") - } -}