Skip to content

Commit f8ae46b

Browse files
committed
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
1 parent 84e60ac commit f8ae46b

2 files changed

Lines changed: 79 additions & 3 deletions

File tree

agentscope-harness/src/main/java/io/agentscope/harness/agent/filesystem/local/LocalFilesystemWithShell.java

Lines changed: 50 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,9 @@
2121
import io.agentscope.harness.agent.filesystem.sandbox.AbstractSandboxFilesystem;
2222
import io.agentscope.harness.agent.workspace.LocalFsMode;
2323
import io.agentscope.harness.agent.workspace.PathPolicy;
24+
import java.io.ByteArrayOutputStream;
2425
import java.io.IOException;
26+
import java.io.InputStream;
2527
import java.nio.charset.Charset;
2628
import java.nio.charset.StandardCharsets;
2729
import java.nio.file.Files;
@@ -335,14 +337,27 @@ public ExecuteResponse execute(
335337

336338
Process proc = pb.start();
337339

340+
// stdout/stderr must be drained concurrently with waitFor: if the child writes
341+
// more than the OS pipe buffer (~4 KB on Windows, 64 KB default on Linux) while
342+
// the parent blocks in waitFor, both sides deadlock and every such command is
343+
// misreported as a timeout (exit 124).
344+
ByteArrayOutputStream stdoutBuf = new ByteArrayOutputStream();
345+
ByteArrayOutputStream stderrBuf = new ByteArrayOutputStream();
346+
Thread stdoutDrainer = drainAsync(proc.getInputStream(), stdoutBuf);
347+
Thread stderrDrainer = drainAsync(proc.getErrorStream(), stderrBuf);
348+
338349
boolean finished = proc.waitFor(effectiveTimeout, TimeUnit.SECONDS);
350+
if (!finished) {
351+
proc.destroyForcibly();
352+
}
353+
joinQuietly(stdoutDrainer);
354+
joinQuietly(stderrDrainer);
339355

340356
Charset outputCharset = outputCharset(osName);
341-
String stdout = new String(proc.getInputStream().readAllBytes(), outputCharset);
342-
String stderr = new String(proc.getErrorStream().readAllBytes(), outputCharset);
357+
String stdout = stdoutBuf.toString(outputCharset);
358+
String stderr = stderrBuf.toString(outputCharset);
343359

344360
if (!finished) {
345-
proc.destroyForcibly();
346361
String msg;
347362
if (timeoutSeconds != null) {
348363
msg =
@@ -432,6 +447,38 @@ private Path resolveExecuteCwd(RuntimeContext rc) {
432447
return namespaced;
433448
}
434449

450+
/**
451+
* Continuously copies a subprocess stream into {@code buf} on a daemon thread so the child
452+
* never blocks on a full OS pipe buffer. Read errors (e.g. the stream closing when the
453+
* process is destroyed on timeout) end the drainer quietly.
454+
*/
455+
private static Thread drainAsync(InputStream in, ByteArrayOutputStream buf) {
456+
Thread t =
457+
new Thread(
458+
() -> {
459+
byte[] chunk = new byte[8192];
460+
int n;
461+
try {
462+
while ((n = in.read(chunk)) != -1) {
463+
buf.write(chunk, 0, n);
464+
}
465+
} catch (IOException ignored) {
466+
// Stream closed because the process was destroyed; nothing to do.
467+
}
468+
});
469+
t.setDaemon(true);
470+
t.start();
471+
return t;
472+
}
473+
474+
private static void joinQuietly(Thread t) {
475+
try {
476+
t.join(5000);
477+
} catch (InterruptedException e) {
478+
Thread.currentThread().interrupt();
479+
}
480+
}
481+
435482
static Charset outputCharset(String osName) {
436483
return outputCharset(osName, System.getProperty("native.encoding"));
437484
}

agentscope-harness/src/test/java/io/agentscope/harness/agent/filesystem/local/LocalFilesystemWithShellTest.java

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,14 @@
1616
package io.agentscope.harness.agent.filesystem.local;
1717

1818
import static org.junit.jupiter.api.Assertions.assertEquals;
19+
import static org.junit.jupiter.api.Assertions.assertFalse;
1920

21+
import io.agentscope.harness.agent.filesystem.model.ExecuteResponse;
2022
import java.nio.charset.Charset;
2123
import java.nio.charset.StandardCharsets;
24+
import java.nio.file.Path;
2225
import org.junit.jupiter.api.Test;
26+
import org.junit.jupiter.api.io.TempDir;
2327

2428
class LocalFilesystemWithShellTest {
2529

@@ -41,4 +45,29 @@ void outputCharset_fallsBackToDefaultWhenWindowsNativeEncodingIsUnavailable() {
4145
Charset.defaultCharset(),
4246
LocalFilesystemWithShell.outputCharset("Windows 10", null));
4347
}
48+
49+
@Test
50+
void execute_outputLargerThanOsPipeBufferCompletesWithoutDeadlock(@TempDir Path tempDir) {
51+
// ~68-72 KB of stdout: beyond the OS pipe buffer (~4 KB on Windows, 64 KB on Linux),
52+
// below the default maxOutputBytes cap. Before stdout/stderr were drained concurrently
53+
// with waitFor, this deadlocked and was misreported as a timeout (exit 124).
54+
int lines = 4000;
55+
String payload = "0123456789abcdef"; // 16 chars per line
56+
boolean windows = System.getProperty("os.name").toLowerCase().contains("win");
57+
String command =
58+
windows
59+
? "for /l %i in (1,1," + lines + ") do @echo " + payload
60+
: "i=0; while [ \"$i\" -lt "
61+
+ lines
62+
+ " ]; do echo "
63+
+ payload
64+
+ "; i=$((i+1)); done";
65+
66+
LocalFilesystemWithShell fs = new LocalFilesystemWithShell(tempDir);
67+
ExecuteResponse resp = fs.execute(null, command, 60);
68+
69+
assertEquals(0, resp.exitCode(), "unexpected exit code, output: " + resp.output());
70+
assertFalse(resp.truncated());
71+
assertEquals(lines, resp.output().split(payload, -1).length - 1);
72+
}
4473
}

0 commit comments

Comments
 (0)