cli: let srt exit after the command when --control-fd is a pipe - #501
Merged
dylan-conway merged 2 commits intoSep 3, 2026
Merged
Conversation
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.
This was referenced Aug 29, 2026
…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.
This was referenced Aug 29, 2026
dylan-conway
approved these changes
Sep 2, 2026
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.
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 withfs.createReadStream, which parks a libuv threadpool thread in a blockingread(2);process.exit()(called when the child exits) waits for the threadpool, sosrtstayed alive until the parent closed the fd or killed it. Reproduced againstdist/cli.jswith apipe(2)read end at fd 3: the wrappedsleep 0.3; echo DONEprinted at 0.7 s andsrtwas still running at 4 s with an empty process tree beneath it; without--control-fdit 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 byprocess.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.SocketthrowsERR_INVALID_FD_TYPE, and the fs stream reads it oneread(2)per datagram, as before); and every fd under Bun, wherenet.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,srtnow 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 inafterEach. Under Bun the next test'sspawncould then race the asynchronous teardown of the fd-3 unix socket and throwFailed to connect— the linux/x86-64 flake seen onmain(Aug 13, Aug 18) and on #499's first run. The tests now subscribe tosrt's exit right after spawning it (ansrtthat 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 andConfig updated from control fd, since the rejection log line echoes the domain too. Two new cases pin the fd kinds thestdio: 'pipe'tests do not reach (Node and Bun implement extra stdio pipes as unix socketpairs): a named pipe frommkfifo, passed as an integer fd with the write end held open for the whole test — whatpipe(2),mkfifoand Pythonpass_fdsembedders handsrt— and a regular file. The valid-update case also runs the samedist/cli.jsunder bun (the runner's own binary) to pin the runtime gate: with the gate removed that leg fails onConfig updated from control fd. Against the old CLI five of the eight fail withsrt did not exit within 4500ms; the four--control-fdpipe 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
srtthemselves see it exit on its own now. The libuv handle switches the fd's open file description to non-blocking mode from the momentsrtopens it, and the flag outlivessrt(Node restores only fds 0-2 at exit); a parent that shares that description (a shellexec 3<fifo, a Pythonpass_fdsof an fd it keeps using) seesEAGAINon its own blocking reads, so handsrta 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 onmain: 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-fdon Windows is unverified there (Node's own stdin uses the same construction).Verified with
tsc,eslint,prettier, andbun teston 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 matchesmain; the suite runs on the four Linux/macOS CI jobs.