From 7fb5e9af966a2caa7b877f8b15467aa05b9c1869 Mon Sep 17 00:00:00 2001 From: zzz-ghost <25565503+zzz-ghost@users.noreply.github.com> Date: Tue, 25 Aug 2026 15:51:19 +0800 Subject: [PATCH] fix(harness): prevent pipe deadlock in LocalFilesystemWithShell.execute Drain stdout/stderr on daemon threads concurrently with Process.waitFor. Previously a child writing more than the OS pipe buffer (~4 KB on Windows, 64 KB on Linux) blocked in write() while the parent blocked in waitFor(), deadlocking until the command was forcibly killed and misreported as a timeout (exit 124). Mirrors the fix already applied to ShellCommandTool in agentscope-core. Adds a regression test that prints ~70 KB from the shell and asserts the command completes with exit 0 and full output. Fixes #2838 --- .../local/LocalFilesystemWithShell.java | 62 ++++++++++++++++++- .../local/LocalFilesystemWithShellTest.java | 29 +++++++++ 2 files changed, 88 insertions(+), 3 deletions(-) diff --git a/agentscope-harness/src/main/java/io/agentscope/harness/agent/filesystem/local/LocalFilesystemWithShell.java b/agentscope-harness/src/main/java/io/agentscope/harness/agent/filesystem/local/LocalFilesystemWithShell.java index 2de590b4d2..960147ec05 100644 --- a/agentscope-harness/src/main/java/io/agentscope/harness/agent/filesystem/local/LocalFilesystemWithShell.java +++ b/agentscope-harness/src/main/java/io/agentscope/harness/agent/filesystem/local/LocalFilesystemWithShell.java @@ -21,7 +21,9 @@ import io.agentscope.harness.agent.filesystem.sandbox.AbstractSandboxFilesystem; import io.agentscope.harness.agent.workspace.LocalFsMode; import io.agentscope.harness.agent.workspace.PathPolicy; +import java.io.ByteArrayOutputStream; import java.io.IOException; +import java.io.InputStream; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; import java.nio.file.Files; @@ -52,6 +54,15 @@ public class LocalFilesystemWithShell extends LocalFilesystem implements Abstrac /** Default timeout in seconds for shell command execution. */ public static final int DEFAULT_EXECUTE_TIMEOUT = 120; + /** Read buffer size (bytes) for the stream drainer threads. */ + private static final int DRAIN_CHUNK_BYTES = 8192; + + /** + * Safety net (millis) for joining drainer threads after the process exits or is destroyed. + * Never reached in practice: process exit closes the streams and ends the drainers at once. + */ + private static final long DRAIN_JOIN_TIMEOUT_MILLIS = 5000; + private final String sandboxId; private final int defaultTimeout; private final int maxOutputBytes; @@ -335,14 +346,27 @@ public ExecuteResponse execute( Process proc = pb.start(); + // stdout/stderr must be drained concurrently with waitFor: if the child writes + // more than the OS pipe buffer (~4 KB on Windows, 64 KB default on Linux) while + // the parent blocks in waitFor, both sides deadlock and every such command is + // misreported as a timeout (exit 124). + ByteArrayOutputStream stdoutBuf = new ByteArrayOutputStream(); + ByteArrayOutputStream stderrBuf = new ByteArrayOutputStream(); + Thread stdoutDrainer = drainAsync(proc.getInputStream(), stdoutBuf); + Thread stderrDrainer = drainAsync(proc.getErrorStream(), stderrBuf); + boolean finished = proc.waitFor(effectiveTimeout, TimeUnit.SECONDS); + if (!finished) { + proc.destroyForcibly(); + } + joinQuietly(stdoutDrainer); + joinQuietly(stderrDrainer); Charset outputCharset = outputCharset(osName); - String stdout = new String(proc.getInputStream().readAllBytes(), outputCharset); - String stderr = new String(proc.getErrorStream().readAllBytes(), outputCharset); + String stdout = stdoutBuf.toString(outputCharset); + String stderr = stderrBuf.toString(outputCharset); if (!finished) { - proc.destroyForcibly(); String msg; if (timeoutSeconds != null) { msg = @@ -432,6 +456,38 @@ private Path resolveExecuteCwd(RuntimeContext rc) { return namespaced; } + /** + * Continuously copies a subprocess stream into {@code buf} on a daemon thread so the child + * never blocks on a full OS pipe buffer. Read errors (e.g. the stream closing when the + * process is destroyed on timeout) end the drainer quietly. + */ + private static Thread drainAsync(InputStream in, ByteArrayOutputStream buf) { + Thread t = + new Thread( + () -> { + byte[] chunk = new byte[DRAIN_CHUNK_BYTES]; + int n; + try { + while ((n = in.read(chunk)) != -1) { + buf.write(chunk, 0, n); + } + } catch (IOException ignored) { + // Stream closed because the process was destroyed; nothing to do. + } + }); + t.setDaemon(true); + t.start(); + return t; + } + + private static void joinQuietly(Thread t) { + try { + t.join(DRAIN_JOIN_TIMEOUT_MILLIS); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + } + } + static Charset outputCharset(String osName) { return outputCharset(osName, System.getProperty("native.encoding")); } diff --git a/agentscope-harness/src/test/java/io/agentscope/harness/agent/filesystem/local/LocalFilesystemWithShellTest.java b/agentscope-harness/src/test/java/io/agentscope/harness/agent/filesystem/local/LocalFilesystemWithShellTest.java index 0efe17b4ad..3f9ff553b9 100644 --- a/agentscope-harness/src/test/java/io/agentscope/harness/agent/filesystem/local/LocalFilesystemWithShellTest.java +++ b/agentscope-harness/src/test/java/io/agentscope/harness/agent/filesystem/local/LocalFilesystemWithShellTest.java @@ -16,10 +16,14 @@ package io.agentscope.harness.agent.filesystem.local; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import io.agentscope.harness.agent.filesystem.model.ExecuteResponse; import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; +import java.nio.file.Path; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; class LocalFilesystemWithShellTest { @@ -41,4 +45,29 @@ void outputCharset_fallsBackToDefaultWhenWindowsNativeEncodingIsUnavailable() { Charset.defaultCharset(), LocalFilesystemWithShell.outputCharset("Windows 10", null)); } + + @Test + void execute_outputLargerThanOsPipeBufferCompletesWithoutDeadlock(@TempDir Path tempDir) { + // ~68-72 KB of stdout: beyond the OS pipe buffer (~4 KB on Windows, 64 KB on Linux), + // below the default maxOutputBytes cap. Before stdout/stderr were drained concurrently + // with waitFor, this deadlocked and was misreported as a timeout (exit 124). + int lines = 4000; + String payload = "0123456789abcdef"; // 16 chars per line + boolean windows = System.getProperty("os.name").toLowerCase().contains("win"); + String command = + windows + ? "for /l %i in (1,1," + lines + ") do @echo " + payload + : "i=0; while [ \"$i\" -lt " + + lines + + " ]; do echo " + + payload + + "; i=$((i+1)); done"; + + LocalFilesystemWithShell fs = new LocalFilesystemWithShell(tempDir); + ExecuteResponse resp = fs.execute(null, command, 60); + + assertEquals(0, resp.exitCode(), "unexpected exit code, output: " + resp.output()); + assertFalse(resp.truncated()); + assertEquals(lines, resp.output().split(payload, -1).length - 1); + } }