Skip to content

cli: let srt exit after the command when --control-fd is a pipe - #501

Merged
dylan-conway merged 2 commits into
anthropics:mainfrom
ronleizrowice-ant:fix/control-fd-exit
Sep 3, 2026
Merged

cli: let srt exit after the command when --control-fd is a pipe#501
dylan-conway merged 2 commits into
anthropics:mainfrom
ronleizrowice-ant:fix/control-fd-exit

Conversation

@ronleizrowice-ant

@ronleizrowice-ant ronleizrowice-ant commented Aug 29, 2026

Copy link
Copy Markdown
Contributor

Split out of #499 at review request; independent of the glob work there.

srt --control-fd <fd> never exited after the wrapped command finished while the parent still held the control pipe open. The fd was read with fs.createReadStream, which parks a libuv threadpool thread in a blocking read(2); process.exit() (called when the child exits) waits for the threadpool, so srt stayed alive until the parent closed the fd or killed it. Reproduced against dist/cli.js with a pipe(2) read end at fd 3: the wrapped sleep 0.3; echo DONE printed at 0.7 s and srt was still running at 4 s with an empty process tree beneath it; without --control-fd it exited at 0.7 s.

Fix: a FIFO or stream socket control fd is read through net.Socket({ fd, readable: true, writable: false }), a libuv stream handle driven by the event loop (unref'd, as the repo does with every long-lived handle; exit is forced by process.exit() when the wrapped command ends). Anything else keeps the fs stream: a regular file (its reads never block); a datagram or seqpacket socket libuv cannot adopt as a stream (net.Socket throws ERR_INVALID_FD_TYPE, and the fs stream reads it one read(2) per datagram, as before); and every fd under Bun, where net.Socket({ fd }) reads nothing and the fs stream does not hold exit. A tty on the control fd keeps the old wait; the docblock says so. With fd 3 held open and a config update written, srt now exits 10 ms after the command.

Tests (test/control-fd.test.ts): the suite used to wait out the hang with a 2 s "timeout safety" that resolved, then SIGKILL the child in afterEach. Under Bun the next test's spawn could then race the asynchronous teardown of the fd-3 unix socket and throw Failed to connect — the linux/x86-64 flake seen on main (Aug 13, Aug 18) and on #499's first run. The tests now subscribe to srt's exit right after spawning it (an srt that dies during the settle sleep is reported by its real exit, not as a hang), wait for it to exit on its own under a 4.5 s cap (about four times the slowest CI job's per-test time, and under bun's 5 s default so the failure names the hang), and assert exit code 0 and Config updated from control fd, since the rejection log line echoes the domain too. Two new cases pin the fd kinds the stdio: 'pipe' tests do not reach (Node and Bun implement extra stdio pipes as unix socketpairs): a named pipe from mkfifo, passed as an integer fd with the write end held open for the whole test — what pipe(2), mkfifo and Python pass_fds embedders hand srt — and a regular file. The valid-update case also runs the same dist/cli.js under bun (the runner's own binary) to pin the runtime gate: with the gate removed that leg fails on Config updated from control fd. Against the old CLI five of the eight fail with srt did not exit within 4500ms; the four --control-fd pipe tests finish in about 0.7 s instead of 2.1 s.

Blast radius, not behind a flag: embedders that kept the control pipe open and killed srt themselves see it exit on its own now. The libuv handle switches the fd's open file description to non-blocking mode from the moment srt opens it, and the flag outlives srt (Node restores only fds 0-2 at exit); a parent that shares that description (a shell exec 3<fifo, a Python pass_fds of an fd it keeps using) sees EAGAIN on its own blocking reads, so hand srt a dedicated pipe end. A control fd that cannot be opened or is not what the caller meant (EBADF; or nothing passed at that number, in which case the fd is one of the runtime's own) behaves as on main: the command still runs with a dead channel, and under node on Linux srt aborts after it — unchanged here, a follow-up if anyone relies on it. Under Bun nothing changes. The new path has no platform guard, and CI's Windows jobs run only the srt-win suites, so --control-fd on Windows is unverified there (Node's own stdin uses the same construction).

Verified with tsc, eslint, prettier, and bun test on macOS, and the full suite in an Ubuntu 24.04 container as an unprivileged user (bubblewrap 0.9.0, as CI installs), where the branch matches main; the suite runs on the four Linux/macOS CI jobs.

srt --control-fd read the control fd with fs.createReadStream, which parks
a threadpool thread in a blocking read(2); process.exit() on the wrapped
command's exit then waited for that thread, so srt stayed alive until the
parent closed the fd or killed it. A pipe or socket fd is now read through
a net.Socket, driven by the event loop and unref'd so it never keeps srt
alive; a regular file keeps the fs stream.

The --control-fd tests waited out that hang with a 2 s timeout and a
SIGKILL, and the next test's spawn could then race Bun's asynchronous
teardown of the fd-3 socket ("Failed to connect"), a flake seen on Linux
CI. They now wait for srt to exit on its own and assert exit code 0, so
the hang is a failing test rather than a timeout.
…agram sockets

net.Socket({ fd }) reads nothing under Bun, whose fs stream does not hold
exit, and throws for a socket libuv cannot adopt as a stream; both keep the
fs stream. The tests subscribe to srt's exit before the settle sleep, cap
the wait at 4.5 s, assert the update was applied rather than echoed, and
add FIFO, regular-file and Bun-runtime cases.
@dylan-conway
dylan-conway merged commit 7c3eb33 into anthropics:main Sep 3, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants