cmux-tui: each connection draws its replies from a share of its socket's budget (#27698 follow-up) by lawrencecchen · Pull Request #28408 · manaflow-ai/cmux

lawrencecchen · 2026-10-30T15:50:57Z

Stacked on #28343 (base fix/pa-b1). Follow-up to its second review, note 1 (#28343 (comment)).

Summary

#28343 removes the 2 s write deadline, so a client that reads late keeps its connection. The cost: a session-socket client that stops reading (a frontend stopped with Ctrl-Z, a stuck agent) keeps its queued replies' bytes from the session socket's shared outbound budget until it reads or hangs up. When that budget is full, other terminal clients' replies can't be made, and their connections close. The app isn't affected, because it has its own budget since #28343.

Now each connection draws its queued replies from its own share of its socket's budget (RenderService::for_connection, OutboundByteBudget::share_of), on the local socket and the WebSocket listener.

Its first bytes get in whenever the pool has room; more only while it then holds no more than the pool still has free. A client that stops reading stops at about half of what is left, and a newcomer's first reply always fits, so no client starves another. This is the same rule as the connection admission's fair share in cmux-tui: each terminal gets a fair share of the terminal connection lane (on top of #27719's 48-of-64 cap) #28343.
Only bytes decide it: no stall signal, no timer, no eviction, no extra descriptor. A share counts every byte its connection holds, including a reply being written, and gives it back to the pool when the reply is dropped.
Trade-off: a single connection now holds at most about half of its socket's pool (64 of 128 MiB per budget) instead of all of it. Past that its reply fails and the connection closes, as at a full pool before. Per-reply limits (32 MiB) are unchanged.

The first version (c3378208dbab) evicted the biggest connection whose send buffer was full. Its review (#28408 (comment)) found that this could end a client that reads: no POLLOUT only means a few KiB are unread. It also found that an eviction did not always free enough, that a writer's in-flight reply was not counted, and that the extra descriptor broke the fd budget (R-L1). 14f202d54a50 replaces it with the share above.

R9 rows: the session socket's shared outbound budget (R-L2: a share for each connection) and R-L5 (no deadline decides anything; only bytes).

Owner: PL-daemon owns cmux-tui-core (OWNERS.md).

Testing

Red first, then the fix: a8d1aa6a97e5 (behavioral red), c3378208dbab (first fix, replaced), 02616aae6106 (unit red for the share rule, with a seam that forwards to the pool), 14f202d54a50 (the share).

a_stalled_client_never_starves_the_other_clients_of_the_shared_budget (server/tests/connection_lanes.rs): a 64 KiB session-socket budget (test-only constructor). One client asks for far more and never reads; after its connection ends (the daemon's ClientDetached event, no timer), a new client and an idle client that reads must both get replies. No read deadline.
a_connection_takes_at_most_half_of_what_is_left_and_a_newcomer_always_fits: on a 100-byte pool, one connection takes at most 50 bytes, two newcomers' first replies fit, the control budget shares the same way, and what a connection gives back returns to the pool.

Ref	Job	Result
behavioral red `a8d1aa6a97e5`	1b680046e46baeab7981cb02	failed on its assertion, both runs: `connection_lanes.rs:556` "a stalled client starved the next client: null"
unit red `02616aae6106`	f8e996d85457a0f99564aefc	pending
green head `14f202d54a50`, Linux (`connection_lanes`)	872ff897b543db17ecdf168b	pending
green head, fleet Mac aws-m4pro-4	376fa5211f5e22794111ea3c	pending
clippy head	0c7ab9f575a14ee4ea0a0951	pending
compile-check head (CmuxNext + tests, app, cmux-tui)	020197d7ac104c430bdadc9d	pending

Observed / Not run

Observed: the behavioral red above; the prepush gate (no new violations).
Not run: a tagged build under load; the Austin's-day harness.

No capture: daemon change with no UI.

Performance

No perf numbers: this is about how outbound memory is shared between clients, not about speed.

Changelog

Fixed: A terminal client that stops reading no longer stops other terminal clients from getting replies from the daemon.

…oad (red, #27698 B1) Austin's nightly-next (2026-10-29): about 80 terminal agents held all 64 daemon connections, 55,162 were refused, the app's among them. Red tests, with behavior-neutral seams (ConnectionBudget at the old 64, ConnectionLane/ConnectionSlot, app_lane_socket_path) so they compile on the base: - an_app_connection_is_served_while_the_session_socket_is_full - one_terminal_opening_many_connections_cannot_starve_another - the_connection_budget_follows_the_open_file_limit - a_client_that_stops_reading_for_a_while_keeps_its_connection - AppLaneConnectionTests (Swift): the app connects on <socket>.app

…res; budget from RLIMIT_NOFILE; no 2 s write kill (#27698 B1) Austin's nightly-next (2026-10-29): about 80 terminal agents held all 64 daemon connections; 55,162 were refused in 2.5 h, the app's among them. The Austin's-day A/B on #28159 showed 64/64 with ~3,700-4,250 refusals per 6-min run. - The app's own socket, `<session socket>.app` (app_lane_socket_path), bound under the same start lock. Each socket has its own budget, so terminals, agents, hooks and the CLI (session socket) can never take the app's connections. The app (LineTransport) connects there first and falls back to the session socket (older daemon, stale file, long path). Chromium: the browser's own IPC channels never compete with renderer connections (render_process_host_impl.cc gives each renderer its own). - A fair share per client on the session socket: the client is the peer's process session (getsid), so a terminal's shell, agent, hooks and cmux calls count as one. A client may take a connection only while it then holds fewer than the socket has left free: one terminal stops at half the lane, and every new client still finds room. Remote entry and WebSocket clients keep the lane budget only (no identity at accept). - The budget comes from the soft RLIMIT_NOFILE the owner already raises at start (owner_start, raise_open_file_limit: hard limit, at most kern.maxfilesperproc / OPEN_MAX): half of the descriptors left after start, 8 per slot (4 descriptors per connection, 2 connections per slot with the hung-up pool), half per socket; on macOS also bounded by kern.num_taskthreads (2 threads per connection). 64 and MAX_SERVER_CONNECTIONS are gone; the parked-connection bound follows it. - No write deadline: STREAM_WRITE_TIMEOUT (2 s) closed a connection whose client read late (a stalled app main thread lost its connection and resynced). Now a client is ended only when its replies overflow the bounded control queue (QueuedSink evicts and shuts the socket); the WebSocket ping never waits on a peer that does not read, and the ping clock shuts a dead peer's socket. Replaces the stopgap split of #27719 (48/64 for terminals).

…not a 3 s sleep (R5, #27698 B1) StalledSocket models the kernel in memory: a daemon write waits while the client's buffer is full, and one that waits when the test declares the stall fails as SO_SNDTIMEO does if a write deadline is set (red on the base: 2 s STREAM_WRITE_TIMEOUT), else keeps waiting (green). The app-lane test reads with no deadline: the daemon answers or refuses with one line.

… prints its own prefix first) (#27698 B1) the_connection_budget_follows_the_open_file_limit failed on both the red and the green fleet runs (16894a02, 3830e38f) at connection_lanes.rs:123: the child printed "connection-budget=30" after libtest's "test <name> ... " on the same line, and the parser only took a line that starts with it.

… .app first; clippy (review of 6aaecf6, #27698 B1) Review FIX (landing/reviews/6aaecf6a9207...md): - Blocker: clippy -D warnings on changed files: SynchronizedTcpStream:: set_write_timeout is dead with no write deadline; same-type casts, unnecessary qualifications and a redundant clone in the tests. - The two sockets shared one RenderService, whose outbound byte budgets a session-socket client that stops reading now holds until its queue overflows (no 2 s write kill). The app's replies could then fail on a full pool and the app's connection close: #27698 by another path. The app socket now has its own RenderService. - cleanup removes <socket>.app before the session socket, so a daemon that binds right after the first removal keeps its fresh .app.

… outbound bytes (review of 6aaecf6, #27698 B1) Regression test for the app socket's own RenderService (0a0796c): spend the session socket's outbound and control budgets, as clients that stopped reading hold them now that no write deadline frees them; a session-socket identify then gets no reply (the test checks it proves something), and the app socket's identify is still answered. ServedSession::new builds the session's two budget sets in one place. Red proof: cp-scratch/pa-b1-red-budget (6aaecf6 + this test, with ServedSession::new serving both sockets from one RenderService).

…on starts without its app socket (red, #27698 B1) Lead review of 5631798, two defects for Austin's load: - every_lane_admits_a_client_at_the_smallest_budget: at the smallest budget (one connection per lane) the fair share refuses every client. - the_session_socket_serves_when_the_app_socket_cannot_be_bound: a directory nobody may remove at <socket>.app makes the whole daemon fail to start.

…ane has room; the daemon starts without its app socket (#27698 B1) - The fair share admitted a further connection only while the client then held fewer than the lane had left free, so a lane of one (the smallest budget an fd limit derives) refused every client. A client's first connection now gets in whenever the lane has room. - A failure to bind <socket>.app (a path that can't be removed, Windows) failed the whole start. The app socket is an addition: the daemon now logs why and serves the session socket alone; the app's LineTransport already falls back to it.

…et clients of the shared budget (red, #27698 follow-up) Second review of #28343, note 1: with no write deadline, a session-socket client that stops reading (a frontend stopped with Ctrl-Z) holds its queued replies' bytes from the session socket's shared outbound budget until it reads or hangs up. When that budget can't fit a reply, the next client is refused (its reply can't be made) and an idle client too. a_stalled_client_never_starves_the_other_clients_of_the_shared_budget: a 64 KiB session budget (test-only ServedSession constructor), a client that asks for far more and never reads; after its connection ends (the daemon's ClientDetached event, no timer), the next client and an idle one must still be answered.

…client that stopped reading (#27698 follow-up) With no write deadline (#28343), a session-socket client that stops reading holds its queued replies' bytes from the socket's shared outbound budget until it reads or hangs up, so other terminal clients could starve. Each local connection now registers with its socket's budgets (StalledEviction, held to the connection's end). When a reply can't fit, OutboundByteBudget ends the holder with the most queued bytes whose client takes no more, then tries once more. Ending it drops its queued replies (their bytes go back) and shuts its socket, so its writer and reader end. Who is stalled is the kernel's signal, not a timer: the socket's send buffer is full (transport::Stream::send_buffer_full, a zero-timeout poll for POLLOUT). A client that reads is never ended this way. The app socket has its own budgets, so this never touches the app for a terminal client. Same idea as BoundedOutbound's largest-stream eviction inside one connection, one level up.

lawrencecchen · 2026-10-30T15:51:09Z

PL-daemon (owner of cmux-tui-core; cmux-tui-platform gets one default trait method, transport::Stream::send_buffer_full): this PR adds stalled-holder eviction to OutboundByteBudget (server/render_service.rs), StalledEviction in server/bounded_outbound.rs, and the registration in server/line_connection.rs. R9 rows: the shared outbound budget (R-L2) and R-L5 (stall = full send buffer, no deadline). Stacked on #28343.

lawrencecchen · 2026-10-30T15:59:23Z

Review (independent review subagent, Claude Opus 5.5): FIX at c337820
reviewer-session: 531b25d4-ae2a-40a0-aa34-ddbc821b9cb1
author-session: pa-b1
R9: R-L1 (a fifth descriptor per connection, but the cap formula still counts 4) and R-L2 (the socket keeps one global reply pool and ends a holder, instead of giving each connection a share)
perf claim: none

Scope: PR #28408, diff 85ccb0b..c337820 (7 files). I read the code, not the PR body. I did not build or run anything. I did not run the fleet jobs again.

Findings

should-fix: the stall signal does not mean "does not read". The central claim is false.
cmux-tui/crates/cmux-tui-platform/src/platform/transport.rs:149-159, bounded_outbound.rs:733-740, render_service.rs:113-118.
A zero-timeout poll that returns no POLLOUT only means "more than a small threshold of bytes is unread now":
- Linux AF_UNIX stream: unix_writable clears POLLOUT when sk_wmem_alloc > sk_sndbuf/4. That is about 53 KB at the default wmem_default of 212992.
- macOS: net.local.stream.sendspace is 8 KiB by default. POLLOUT clears when sbspace(so_snd) < sb_lowat.
A healthy client that receives a large reply (a vt-state replay, an output burst, a remote viewer behind a slow link) shows "full" most of the time, because its writer thread is blocked in write(). The budget fills exactly when large transfers are in flight, so healthy heavy readers are the likely victims. Also, max_by_key (render_service.rs:159) picks the healthy reader over a truly stalled client that holds fewer bytes. The evicted client must reconnect and resync everything, which is the cmux NEXT: agents in terminals can fill the daemon's 64 connections and lock the app out #27698 harm.
- The doc line "A client that reads is never ended this way" (render_service.rs:117) is wrong, and so is the commit message.
- Root cause: one connection may hold more than the whole pool. OUTBOUND_CONNECTION_BYTE_CAPACITY is 256 MiB (server.rs:2592), but OUTBOUND_GLOBAL_BYTE_CAPACITY is 128 MiB (server.rs:2587).
- Principled fix (R-L2): give each connection a share of its socket's budget, charged for both queued and in-flight bytes. A stalled client then fills only its own share and its own queue, and the existing QueuedSink overflow rule ends it. No stall signal is needed.
- Fix if eviction stays: a holder counts as stalled only when its socket is full and its writer made no progress since the previous shortage. The writer thread increments a bytes-written counter; this is an event-driven second chance with no timer.
should-fix: eviction is not tied to the shortfall, and concurrent shortages evict too many clients. render_service.rs:130-162.
- (a) It evicts even when the victim's queued bytes cannot cover the request. Example: reserved_control_writer (connection_frame.rs:244) reserves 32 MiB for each vt-state response, so 4 concurrent ones fill the 128 MiB control budget. A 5th then evicts a client, its retry still fails, and the victim died for nothing.
- (b) stalled_bytes returns regular + control bytes, whichever budget is short. Evicting a holder of mostly outbound bytes frees little control budget. Also, push_ordered_terminal puts control-budget text in the regular queue.
- (c) N threads that fail claim at the same time each evict one holder. After abort, the first victim reports 0 bytes, so the next thread picks the next-biggest holder. Nothing serializes this.
- Fix: run eviction under one lock per budget. Evict holders of that budget's bytes only until the shortfall is covered, and claim again after each eviction.
should-fix: the writer's in-flight reply is not counted, and it is not freed when try_retain tries again. bounded_outbound.rs:733-740, line_connection.rs:51-52.
- The Arc<BudgetedText> that the writer thread holds in write_line_outbound_item, while blocked on the full socket, is not in regular_bytes/control_bytes.
- That item is released only after the writer's write() returns EPIPE, which is asynchronous and comes after the single retry.
- A stalled client whose bytes sit in one large in-flight reply (up to 32 MiB) with an empty queue reports None and can never be evicted. Four such clients pin the whole 128 MiB pool, so the starvation this PR fixes is still possible.
- Fix: keep the in-flight size in BoundedOutboundState. The writer sets it when it pops an item and clears it after the write. Count it, and make the release observable (an event, not a timer) before the claim is tried again.
should-fix (R9 R-L1): one more file descriptor per connection, with no change to the cap formula. line_connection.rs:41.
- The new watched duplicate is a fifth descriptor per connection. diagnostics/connection_budget.rs:40 sets FDS_PER_CONNECTION = 4 and names exactly four: the socket, the writer's duplicate, the control duplicate and the hang-up watcher's duplicate.
- The connection cap from RLIMIT_NOFILE now under-counts by 25% and uses part of the half reserved for terminal hosts, the journal and the listeners.
- Fix: reuse the existing control duplicate (SinkControl::Unix) or the peer-watcher's duplicate for the holder's poll and shutdown. Or raise FDS_PER_CONNECTION to 5 and update its doc.
should-fix: the regression test does not check the stall signal. server/tests/connection_lanes.rs:523-560.
- The idle client holds 0 queued bytes, so (bytes > 0) excludes it (bounded_outbound.rs:739), not send_buffer_full. The test passes with send_buffer_full hard-coded to true.
- The eviction in the test is self-eviction, triggered by the stalled client's own reply. Two cases are not tested: another client's reply evicting the stalled one, and "a reading client with queued bytes is spared". A test where a reading client is mid-transfer when the budget fills would show finding 1.
- Otherwise the red and green are valid:
  - The red is deterministic: MessageWriter::close does not shut the socket, so the blocked writer holds the bytes until the test drops the stalled client.
  - The green does not depend on timing: abort frees the queued bytes before disconnect_client publishes ClientDetached.
note (R5): connection_lanes.rs:494. The 5 s read timeout returns Value::Null, the same value as a close, so a very slow host could fail a green run as "starved". A red run ends by EOF without the timeout, so the timeout can go. ask() in the same file uses no timeout.
note (R-L3/R-L5): an evicted client gets EOF with no typed reason, and its queued replies are dropped silently. This is the same as the existing evict_on_overflow.
note (scope):
- WebSocket connections use their own RenderService for each listener (websocket_listener.rs:143) and do not register, so they leave no gap in the session socket's budget. The same starvation can still happen among the WebSocket clients of one listener.
- Remote-entry connections do register, through serve_line_connection (remote_entry.rs:174, 248). Remote viewers behind slow links are the clients most exposed to finding 1.

Checked and found fine

Lock order: no caller of try_retain holds a BoundedOutbound state lock. All push_* calls take text that is already serialized. project_api_revision, project_conversation_tabs and the reserved writers serialize before the sink.
The holders mutex is released before stalled_bytes/evict run.
Self-eviction is safe: the handler holds no state lock of its own while it serializes, and its push then fails with BrokenPipe.
abort fires the stream interrupts under the victim's state lock, and the wakers then take mailbox, attach-tap, render-tap and journal locks. This order already existed (QueuedSink::abort, disconnect paths), and no serializer holds those locks.
evict ends the connection:
- abort clears the queues, and dropping each BudgetedText releases its bytes at once.
- shutdown(Both) on the duplicate wakes the blocked writer (EPIPE) and the reader (EOF) on Linux and macOS. This is the same mechanism as evict_on_overflow.
- Then disconnect_client runs, the writer thread joins, and the permit is dropped.
- The holder Arc is dropped at the end of serve_line_connection, and its Weak entry is pruned.
- I found no thread, slot or registration leak.
The remote entry's Prefixed::try_clone_box returns the inner UnixStream, so the signal works there.
A poll error (-1) counts as "not full", which is the safe side.
The #[cfg(test)] constructor with_small_session_budgets_for_test in listen.rs is acceptable.
The stall check is racy (poll, then a lock), but that race does not matter compared with finding 1.
No perf claim is made.

…s socket's reply budget; a newcomer always fits (red, review of c337820) Review FIX on #28408 (landing/reviews/c3378208dbab...md): evicting the biggest connection whose send buffer is full can end a client that reads (no POLLOUT only means a few KiB are unread), can evict without freeing enough, and misses a writer's in-flight reply. The principled design it names: a share of the socket's budget per connection, no stall signal, no eviction. a_connection_takes_at_most_half_of_what_is_left_and_a_newcomer_always_fits is red on this commit: RenderService::for_connection / OutboundByteBudget:: share_of are seams that forward to the pool with no share rule yet. The stalled-client test's SessionClient reads with no deadline (R5).

…socket's budget; no stall eviction (review of c337820, #27698 follow-up) Replaces the eviction in c337820, which the review found wrong: no POLLOUT only means a few KiB are unread, so a client that reads could be ended; an eviction did not always free enough; a writer's in-flight reply was not counted; and the watched descriptor made five per connection against the budget's four (R-L1). Now each line and WebSocket connection gets its own share of its socket's outbound and control budgets (RenderService::for_connection, OutboundByteBudget::share_of). Its first bytes get in whenever the pool has room; more only while it then holds no more than the pool still has free. A client that stops reading stops at about half of what is left, and a newcomer's first reply always fits, so no client starves another: the same rule as the connection admission's fair share. A share counts every byte its connection holds, a reply being written included, and gives it back to the pool when dropped. Only bytes decide it: no stall signal, no timer, no eviction, no extra descriptor. A single connection now holds at most about half of its socket's pool (64 of 128 MiB per budget) instead of all of it; past that, its reply fails and the connection closes, as at a full pool before. Per-reply limits (32 MiB) are unchanged.

lawrencecchen · 2026-11-01T10:00:27Z

Conflict wave: I did not merge feat-cmux-next into this branch. This PR is stacked on an earlier head of #28343 (fix/pa-b1), and that design did not land. The #28343 that merged (96428aa, 06:09Z) builds on #27719's 48-of-64 terminal lane cap: ConnectionStats.terminal_active, terminal_clients and try_claim_terminal/claim_terminal_slot with a per-terminal fair share. This PR's diagnostics/connection_budget.rs (ConnectionBudget, ConnectionLane, ConnectionSlot) replaces the same ConnectionStats admission. There are 11 conflict blocks in diagnostics.rs, connection_leases.rs, bounded_outbound.rs, listen.rs and restoring.rs, and every one is in that admission and outbound code. The merge needs a design decision, not a mechanical resolution.

Question for the author: should the per-connection outbound share (RenderService::for_connection, OutboundByteBudget::share_of) be ported onto feat's terminal-lane model, keeping terminal_active/terminal_clients for admission and adding only the byte share? Or does ConnectionBudget replace feat's lane model? If you choose the port, I can do it as a new commit on this branch (a merge, no rebase). The label is unchanged (risk:high), and I did not close the PR.

lawrencecchen added 11 commits October 30, 2026 13:05

Merge remote-tracking branch 'origin/feat-cmux-next' into fix/pa-b1

85ccb0b

lawrencecchen added the risk:high Risk tier (docs/ci/risk-tiers.md) label Oct 30, 2026

lawrencecchen temporarily deployed to artifacts October 30, 2026 15:59 — with GitHub Actions Inactive

lawrencecchen temporarily deployed to artifacts October 30, 2026 16:01 — with GitHub Actions Inactive

lawrencecchen added 2 commits October 30, 2026 16:01

lawrencecchen changed the title ~~cmux-tui: when a socket's reply budget is full, end the biggest client that stopped reading (#27698 follow-up)~~ cmux-tui: each connection draws its replies from a share of its socket's budget (#27698 follow-up) Oct 30, 2026

lawrencecchen temporarily deployed to ci-routing-agent October 30, 2026 16:06 — with GitHub Actions Inactive

lawrencecchen temporarily deployed to ci-routing-agent October 30, 2026 16:11 — with GitHub Actions Inactive

Base automatically changed from fix/pa-b1 to feat-cmux-next November 1, 2026 06:09

Navigation Menu

Repository navigation

cmux-tui: each connection draws its replies from a share of its socket's budget (#27698 follow-up) - #28408

cmux-tui: each connection draws its replies from a share of its socket's budget (#27698 follow-up)#28408
lawrencecchen wants to merge 13 commits into
feat-cmux-nextfrom
fix/pa-b1-stalled-evict

lawrencecchen commented Oct 30, 2026 •

edited

Loading

Uh oh!

lawrencecchen commented Oct 30, 2026

Uh oh!

lawrencecchen commented Oct 30, 2026

Uh oh!

lawrencecchen commented Nov 1, 2026

Uh oh!

This branch was previously deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

lawrencecchen commented Oct 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Testing

Observed / Not run

Performance

Changelog

Uh oh!

lawrencecchen commented Oct 30, 2026

Uh oh!

lawrencecchen commented Oct 30, 2026

Findings

Checked and found fine

Uh oh!

lawrencecchen commented Nov 1, 2026

Uh oh!

This branch was previously deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

lawrencecchen commented Oct 30, 2026 •

edited

Loading