Skip to content

fix(envd): buffer subscriber channels to prevent ghost-subscriber fan-out wedge - #3736

Open
AdaAibaby wants to merge 1 commit into
e2b-dev:mainfrom
AdaAibaby:fix/multiplex-ghost-subscriber-wedge
Open

AdaAibaby wants to merge 1 commit into
e2b-dev:mainfrom
AdaAibaby:fix/multiplex-ghost-subscriber-wedge

Conversation

@AdaAibaby

Copy link
Copy Markdown
Contributor

Problem

After a sandbox is paused and resumed, envd can keep a subscriber whose client TCP connection is already dead — the client's FIN arrived after the VM snapshot was taken. That subscriber's consumer goroutine is wedged inside stream.Send, waiting for TCP's retransmit timeout: ~924 s (~15 min) at the kernel default tcp_retries2=15.

MultiplexedChannel used unbuffered subscriber channels. The fan-out loop serialises over all subscribers; when it reaches the ghost, both arms of its select block:

  • case s.ch <- v: — the consumer is stuck in stream.Send, so nobody is reading s.ch.
  • case <-s.done: — the consumer never returns, so its deferred cancel() never runs and done is never closed.

The whole fan-out stalls for every subscriber for the full TCP timeout. This is exactly #3292: stdout/stderr goes silent for ~15 min, then suddenly resumes.

The existing TestMultiplexedChannel_AbandonedConsumer* tests don't catch this: they model a consumer that stops reading but then calls cancel(), which closes done and lets the fan-out unblock via case <-s.done. A real ghost never returns from stream.Send, so cancel() never runs.

Fix — multiplex.go only

  1. Buffered subscriber channels (subscriberBufSize = 64) absorb normal bursts past a briefly-slow consumer.
  2. Non-blocking overflow path (default:): a consumer that falls a full buffer behind is treated as a ghost and dropped —
    • cancel() closes done so later fan-out iterations skip it via isCancelled();
    • closeCh() closes s.ch so the ghost's own for range consumer exits (no goroutine leak).
    • Both are sync.Once-guarded (once for done, a separate chOnce for ch), so double-close is impossible and the two are independent.

Healthy subscribers are untouched — their sends are buffered or complete immediately.

Tests — multiplex_ghost_test.go

Three regression tests for the uncovered case (ghost that never reads and never cancels):

Test What it proves
GhostNeverReadingDoesNotWedgeFanOut Flood subscriberBufSize+10 past a ghost; the healthy subscriber still receives every value, in order, within the timeout.
GhostChannelClosedAfterOverflow After overflow the ghost channel is closed, so a for range consumer terminates instead of leaking.
MultipleHealthyUnaffectedByOneGhost 3 healthy subscribers all receive the full stream alongside one ghost.

All three fail (producer back-pressured to timeout on send 3) against the current unbuffered implementation, and pass under -race with the fix. go build ./... and go vet are clean.

Scope

Deliberately scoped to this one fix. The earlier #3344 also bundled a separate stream.Send write-deadline mechanism (writedeadline.go) plus connect/start/service wiring — a distinct concern, left out here so this change stays small and reviewable.

Closes #3292. Supersedes #3344 (closed for inactivity / merge drift; maintainer invited a fresh PR against current main).

…rocess-stream fan-out

A subscriber whose client TCP connection died after a sandbox resume (its
FIN arrived after the VM snapshot) leaves its consumer goroutine wedged
inside stream.Send until the kernel's retransmit timeout fires — ~924s
(~15 min) at the default tcp_retries2=15.

MultiplexedChannel used unbuffered subscriber channels. The fan-out loop
serialises over all subscribers; at the ghost both `case s.ch <- v` (nobody
reading) and `case <-s.done` (never cancelled, because the consumer never
returns to run its deferred cancel) block, stalling delivery to every other
subscriber for the full timeout. This is e2b-dev#3292: stdout/stderr goes silent
for exactly ~15 min, then resumes.

Fix, scoped to multiplex.go:

  1. Buffered subscriber channels (subscriberBufSize = 64) absorb normal
     bursts past a briefly-slow consumer.
  2. A non-blocking overflow path (`default:`) treats a consumer that falls
     a full buffer behind as a ghost and drops it: cancel() so later
     iterations skip it, closeCh() so its own goroutine's range exits. Both
     are sync.Once-guarded, so double-close is impossible. chOnce is kept
     separate from once so cancel (done) and closeCh (ch) stay independent.

Healthy subscribers are untouched — their sends are buffered or complete
immediately.

Adds multiplex_ghost_test.go with three regression tests for the case the
existing Abandoned* tests do not cover — a ghost that never reads AND never
cancels:
  - GhostNeverReadingDoesNotWedgeFanOut: flood subscriberBufSize+10 past a
    ghost; the healthy subscriber still receives every value in order.
  - GhostChannelClosedAfterOverflow: after overflow the ghost channel is
    closed, so a ranging consumer terminates instead of leaking.
  - MultipleHealthyUnaffectedByOneGhost: 3 healthy subscribers all receive
    the full stream alongside one ghost.
All three fail (producer back-pressured to timeout) against the unbuffered
implementation and pass under -race with the fix.

Scoped to this one fix. The original e2b-dev#3344 also bundled a separate
stream.Send write-deadline mechanism (writedeadline.go) and connect/start
wiring; that is a distinct concern and is left out here so this change stays
reviewable. Closes e2b-dev#3292. Supersedes e2b-dev#3344.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

envd: a stale process-stream subscriber can wedge stdout fan-out for all subscribers for ~15 minutes after sandbox resume

2 participants