Repository navigation
fix: close the launcher's descriptors in the sandbox intermediate - #23
Merged
Merged
Conversation
A confined run on Linux stopped when the harness wrote more than 64 KiB to standard output. The process that becomes pid 1 forks from an intermediate process. The intermediate does not exec, so it kept open all the descriptors of the launcher. One of these is the write end of the exec-error socket that std reads in spawn. Thus spawn did not return until the run ended, nothing read the capture pipe, and the harness stopped when the pipe was full. Now the intermediate closes all descriptors from 3 up after its fork (close_range, or a close loop on a kernel without it). Pid 1 keeps its own copy of the exec-error socket, thus a failure before its exec still gets to spawn. Add a launch test: a confined prompted run whose harness writes 200000 bytes completes in time, and all of the output gets to the launcher. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
sstriker
requested review from
andrewmusselman
and
a balanced review from Copilot
October 25, 2026 00:20
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A confined launch with a prompt (
amem launch --prompt) stopped forever when the harness wrote more than 64 KiB to stdout. The harness container survey in #2 found this.The cause
A confined launch forks twice. The second fork (
become_pid_one) makes pid 1 of the new pid namespace. Its parent, the intermediate, never runs exec. It only waits for pid 1 and exits.std::process::Command::spawnreads its exec-error channel until end of file. With this toolchain, that channel is aSOCK_SEQPACKETsocket pair. The intermediate held a copy of every descriptor of the launcher, that channel included. Sospawn()returned only at the end of the run. Until then, nothing read the capture pipe. When the pipe was full, the harness stopped in its write.This was seen on a hung launch: the launcher was in
recvon the socket pair, and the intermediate held the other end.The change
crates/sandbox/src/linux.rs: the intermediate closes every descriptor from 3 up, right after the fork (close_from_three). It usesclose_range, or acloseloop below the open-files limit on an older kernel. Both are system calls only, with no allocation.spawnas an error.Test
crates/memory/tests/launch.rs:a_confined_prompted_run_captures_more_than_a_pipe_holds. A confinedamem launch --prompt hello --no-handoffruns a harness that printsbegin, 200000 bytes ofa, andend. The test asserts exit 0 and the exact output. After 90 s it stops the launch and fails, so a regression does not hang CI.Failing first
Against
main'slinux.rs:the launch had not returned after 90 s, with 0 bytes out.With the fix, the launch tests pass in 2.84 s.
make checkafter the merge ofmain: 656 tests, 656 passed, 6 skipped.Adversarial pass
A fresh agent attacked this branch and the stacked branch
fix/killed-launch-cleanup. WithCI=1, the confined tests ran. For this branch it found these points:close_from_threeremoved, the new test fails at its 90 s bound.The other findings are for the stacked branch. They are fixed there, before its own pull request.
Posted by Claude on behalf of @sstriker