fix: stop leaking a tokio runtime per .cat/.last, consolidate read commands - #146
Merged
Conversation
cablehead
force-pushed
the
fd-leak-repro
branch
from
August 5, 2026 20:46
d090190 to
1c44248
Compare
cablehead
force-pushed
the
fd-leak-repro
branch
from
August 5, 2026 20:53
1c44248 to
e9afa6a
Compare
.cat --follow spawns an OS thread that builds a fresh tokio Runtime (src/nu/commands/cat_stream_command.rs:147) and block_on's a loop forwarding store frames to a ListStream. The runtime owns an eventfd (anon_inode) + wakeup socket and is only dropped when block_on returns. In --follow mode the read() future parks on the broadcast receiver awaiting the next append. When the ListStream consumer is dropped the spawned thread stays parked inside receiver.recv().await and never observes that the std mpsc tx is disconnected (std mpsc only reports Disconnected on the next send, which never happens while parked). The thread, its runtime, eventfd, and socket leak for the life of the process -- one socket+eventfd pair per invocation, matching the prod EMFILE / growing socket+anon_inode pairs. test_cat_stream_fd_leak drives .cat --follow through the real engine, takes the historical frame, drops the stream, and asserts /proc/self/fd stays bounded. Fails on current code: fd count grows ~2.8/iter (before=18 -> after=158 over 50 runs).
…mmands .cat/.last spawned a fresh tokio Runtime per call and forwarded frames over a std::mpsc; in --follow mode the forwarding thread parked on the broadcast recv and never noticed the consumer drop, so the runtime (eventfd + socket) leaked -- EMFILE over time. - store::read is now sync; its follow/heartbeat tasks select on tx.closed() so they cancel when the receiver drops. Store carries a runtime Handle. - .cat/.last stream via blocking_recv on the shared runtime; no per-call runtime. - Collapse the four read commands into one .cat and one .last; drop the ReadMode split.
The follow task is cancelled asynchronously (via tx.closed() on the shared runtime) after the stream drops, so a loaded CI runner shows in-flight fds if sampled immediately. Poll until the count settles instead; a real leak never settles and still fails after the deadline.
/proc/self/fd is process-wide and cargo runs tests in parallel, so measuring it in-suite races unrelated tests opening fds (CI showed the count ballooning during an idle settle sleep). Run the measurement in a spawned, single-test process instead; drop the settle-poll that only widened the noise window.
Historical (non-follow) .cat/.last collected every frame into a Vec via drain_frames before returning. They now stream lazily through a ListStream over rx.blocking_recv(), the same shape the follow path already uses, so a large backlog is no longer held in memory. blocking_recv panics if called on a tokio runtime thread, so two consumers that evaluated on an async thread moved onto a std::thread: the actor config body eval (parse_config) and the xs eval HTTP endpoint (handle_eval). The config eval threads its stack back out so the later merge_env still sees the env. drain_frames is deleted.
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.
Problem
.cat/.lastleaked a file descriptor on every call. A long-livedstacks2099accumulated them until it hit the open-file limit and died:Each call spawned a thread that built a fresh
tokio::runtime::Runtimeand forwarded frames over astd::sync::mpsc. A tokio runtime holds an eventfd and a wakeup socket. With--followthe thread parked on the broadcast receiver waiting for the next append. When the consumer went away the parked thread never noticed, because a std mpsc only reports disconnection on the next send and that send never came. So the runtime and its two descriptors stayed alive for the life of the process. One pair leaked per call.Fix
store::readis now sync. Its follow task selects ontx.closed()and exits when the receiver drops.Storecarries a runtime handle, and.cat/.laststream frames throughrx.blocking_recv()on that shared runtime. Nothing builds a runtime per call.Consolidation
The read commands were four: a
ReadMode::Streampair and aReadMode::Plainpair. That split is gone. One.catand one.lastserve every caller now.Historical (non-
--follow) reads used to collect the whole backlog into aVecbefore returning. They stream lazily now, like follow. That meant moving two consumers off the tokio runtime thread, sinceblocking_recvpanics there: the actor config-body eval and thexs evalHTTP endpoint. Both run on a plainstd::threadand hand their result back.Test
test_cat_stream_fd_leakruns.cat --followin a loop and watches/proc/self/fd. It runs in its own spawned process, becausecargo testis parallel and the fd count is process-wide. Before the fix the count grew about 2.8 descriptors per iteration. Now it stays flat.