mirror of
https://github.com/bytedance/deer-flow.git
synced 2026-09-25 14:06:18 +00:00
fix(sandbox): wrap implicit-session probe commands in a subshell to prevent wedged shell (#1433 family) (#5546)
The AIO sandbox's implicit persistent shell session hangs forever when a
command containing a bare 'exit' is executed: exit kills the session's
shell process, and the server's response path for that exec_command
request never completes. Verified against a standalone all-in-one-sandbox
1.11.0 container via the raw SDK:
shell.exec_command('seq 1 1000 | head -n 5') # OK (not a SIGPIPE issue)
shell.exec_command('echo x; exit 0') # HANGS every time
shell.exec_command('echo after') # OK (server recreates shell)
shell.exec_command('( echo x; exit 0 )') # OK, exit code propagates
remote_list_dir_command and remote_search_command both end their probe
scripts with a bare 'exit' (to propagate the find/grep status code), so
every list_dir/grep/glob call deterministically wedges the session —
this is the root cause behind parallel [ls, bash] tool calls deadlocking
an entire run (same defect family as #1433 and #5128).
Fix: keep 'set +e' as the outermost prefix (pinned by existing tests) and
wrap the rest of each probe script in a subshell, so exit only terminates
the subshell. Output and exit codes propagate identically.
Tests: 116 passed (backend/tests/test_aio_sandbox.py, test_remote_list_dir.py,
test_remote_search.py); one endswith assertion updated to the wrapped form.
Co-authored-by: mad_max <mad_max@coscoshipping.local>
This commit is contained in:
parent
bd995a6a26
commit
dba3967177
@ -34,14 +34,20 @@ def remote_list_dir_command(path: str, max_depth: int, *, limit: int = _LIST_LIM
|
||||
# a login-profile ``set -e`` so a failing find still records $?. End with
|
||||
# ``exit`` of that status (126 if the file is missing): the last command
|
||||
# would otherwise be ``rm``, whose 0/1 is not find's status.
|
||||
# ``set +e`` stays outermost (pinned by existing tests); the rest runs in a
|
||||
# ( ... ) subshell: a bare ``exit`` in the implicit persistent session kills
|
||||
# the session's shell process and the AIO server's response path for that
|
||||
# request hangs forever (verified: bare ``exit 0`` always wedges; a subshell
|
||||
# ``exit`` only kills the subshell, the session survives, and the exit code
|
||||
# and output propagate unchanged).
|
||||
return (
|
||||
f"set +e; if [ ! -e {quoted} ]; then printf '%s\\n' {_STATUS_PREFIX}{_MISSING_ROOT}; exit 1; fi; "
|
||||
f"set +e; ( if [ ! -e {quoted} ]; then printf '%s\\n' {_STATUS_PREFIX}{_MISSING_ROOT}; exit 1; fi; "
|
||||
f"_st=/tmp/df_find_$$; "
|
||||
f"{{ find -H {quoted} -maxdepth {depth} \\( -type f -o -type d \\) 2>/dev/null; "
|
||||
f'echo $? > "$_st"; }} | head -n {n}; '
|
||||
f'st=$(cat "$_st" 2>/dev/null); '
|
||||
f"printf '\\n%s\\n' {_STATUS_PREFIX}$st; "
|
||||
f'rm -f "$_st"; exit "${{st:-126}}"'
|
||||
f'rm -f "$_st"; exit "${{st:-126}}" )'
|
||||
)
|
||||
|
||||
|
||||
|
||||
@ -54,11 +54,16 @@ def remote_search_command(search: str, root: str, *, limit: int) -> str:
|
||||
quoted = shlex.quote(root)
|
||||
# One extra line is the truncation signal; the parser drops it.
|
||||
n = int(limit) + 1
|
||||
# ``set +e`` stays outermost (pinned by existing tests); the rest runs in a
|
||||
# ( ... ) subshell: a bare ``exit`` in the implicit persistent session kills
|
||||
# the session's shell process and the AIO server's response path hangs
|
||||
# forever; a subshell ``exit`` only kills the subshell, so the session
|
||||
# survives and the exit code propagates unchanged.
|
||||
return (
|
||||
f"set +e; if [ ! -e {quoted} ]; then printf '%s\\n' {_STATUS_PREFIX}{_MISSING_ROOT}; exit 0; fi; "
|
||||
f"set +e; ( if [ ! -e {quoted} ]; then printf '%s\\n' {_STATUS_PREFIX}{_MISSING_ROOT}; exit 0; fi; "
|
||||
f'_st=/tmp/df_search_$$; {{ {search}; echo $? > "$_st"; }} | head -n {n}; '
|
||||
f'st=$(cat "$_st" 2>/dev/null); rm -f "$_st"; '
|
||||
f"printf '\\n%s\\n' {_STATUS_PREFIX}\"$st\"; exit 0"
|
||||
f"printf '\\n%s\\n' {_STATUS_PREFIX}\"$st\"; exit 0 )"
|
||||
)
|
||||
|
||||
|
||||
|
||||
@ -89,6 +89,10 @@ def test_command_records_find_status_after_head() -> None:
|
||||
assert command.index("find -H ") < command.index("head -n")
|
||||
assert command.index("head -n") < command.rindex("__DF_FIND_STATUS__:")
|
||||
assert 'exit "${st:-126}"' in command
|
||||
# Mirror pin (review on PR #5546): the exit must stay subshell-wrapped — a
|
||||
# bare top-level exit wedges the implicit persistent session. A silent
|
||||
# revert of the subshell wrap must fail this test.
|
||||
assert command.endswith('exit "${st:-126}" )')
|
||||
|
||||
|
||||
def _run_list_dir_script(command: str, *, env: dict[str, str] | None = None) -> subprocess.CompletedProcess[str]:
|
||||
|
||||
@ -121,7 +121,11 @@ def test_command_checks_root_first_and_records_status_after_head() -> None:
|
||||
# One line past the limit is what lets the parser tell a full result from a cut one.
|
||||
assert command.index("[ ! -e ") < command.index("grep ") < command.index("head -n 451")
|
||||
assert command.index("head -n 451") < command.rindex("__DF_SEARCH_STATUS__:")
|
||||
assert command.endswith("exit 0")
|
||||
# The command ends with a subshell-wrapped exit: a bare top-level exit would
|
||||
# kill the implicit persistent session's shell and wedge the AIO server
|
||||
# response for that request (verified); a subshell exit only kills the
|
||||
# subshell and the exit code propagates unchanged.
|
||||
assert command.endswith("exit 0 )")
|
||||
|
||||
|
||||
# ── real POSIX sh ─────────────────────────────────────────────────────────
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user