Skip to content

fix: stop leaking a tokio runtime per .cat/.last, consolidate read commands - #146

Merged
cablehead merged 5 commits into
mainfrom
fd-leak-repro
Aug 5, 2026
Merged

fix: stop leaking a tokio runtime per .cat/.last, consolidate read commands#146
cablehead merged 5 commits into
mainfrom
fd-leak-repro

Conversation

@cablehead

@cablehead cablehead commented Aug 5, 2026

Copy link
Copy Markdown
Owner

Problem

.cat/.last leaked a file descriptor on every call. A long-lived stacks2099 accumulated them until it hit the open-file limit and died:

thread panicked at src/nu/commands/cat_stream_command.rs:136:
Failed to create tokio runtime: Too many open files (os error 24)

Each call spawned a thread that built a fresh tokio::runtime::Runtime and forwarded frames over a std::sync::mpsc. A tokio runtime holds an eventfd and a wakeup socket. With --follow the 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::read is now sync. Its follow task selects on tx.closed() and exits when the receiver drops. Store carries a runtime handle, and .cat/.last stream frames through rx.blocking_recv() on that shared runtime. Nothing builds a runtime per call.

Consolidation

The read commands were four: a ReadMode::Stream pair and a ReadMode::Plain pair. That split is gone. One .cat and one .last serve every caller now.

Historical (non---follow) reads used to collect the whole backlog into a Vec before returning. They stream lazily now, like follow. That meant moving two consumers off the tokio runtime thread, since blocking_recv panics there: the actor config-body eval and the xs eval HTTP endpoint. Both run on a plain std::thread and hand their result back.

Test

test_cat_stream_fd_leak runs .cat --follow in a loop and watches /proc/self/fd. It runs in its own spawned process, because cargo test is parallel and the fd count is process-wide. Before the fix the count grew about 2.8 descriptors per iteration. Now it stays flat.

.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.
@cablehead
cablehead merged commit 3533e96 into main Aug 5, 2026
7 of 10 checks passed
@cablehead
cablehead deleted the fd-leak-repro branch August 5, 2026 21:56
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.

1 participant