daemon: make connAdapter read deadlines real, stop leaking registry timers - #435
Open
TeoSlayer wants to merge 1 commit into
Open
daemon: make connAdapter read deadlines real, stop leaking registry timers#435TeoSlayer wants to merge 1 commit into
TeoSlayer wants to merge 1 commit into
Conversation
…imers
connAdapter's deadline setters were stubs:
func (a *connAdapter) SetReadDeadline(t time.Time) error { return nil }
They stored nothing and reported success. Anything that set a read deadline
got no deadline and no error — a silent lie, which is worse than not
implementing the method, because callers cannot detect the failure.
Read consequently blocked on <-a.conn.RecvBuf forever. A peer that opened a
stream and then went silent pinned its handler goroutine plus the whole
Connection: a 512-slot RecvBuf, a 256-slot SendBuf and three goroutine
stacks, roughly 43 KiB each.
On 2026-07-28 that filled the daemon's own documented bound —
DefaultMaxTotalConnections (65536) x ~43 KiB = ~2.8 GB — across 431 service
agents on a 256 GB host, causing system-wide kernel OOM. The agents were
reaped, restart-looped, stopped heartbeating, and the registry expired every
registration, so the whole fleet went unresolvable. The daemon was not
leaking past a bound; it was filling its bound and sitting there, which is
why growth was ~100x and then plateaued. Idle daemons elsewhere were fine at
17 MB after 24 weeks, identifying this as per-request retention.
Changes:
- SetReadDeadline now stores an absolute deadline (atomic; it may be set
from a goroutine other than the one parked in Read), and Read selects on
RecvBuf against a timer, returning a net.Error with Timeout() == true.
- SetDeadline delegates to SetReadDeadline instead of silently doing
nothing. SetWriteDeadline stays a no-op but is now documented as such —
Write already bounds itself via connAdapterWriteDeadline.
- withRegistryDeadline no longer leaks a timer per call. time.After pins
its timer for the full 8s duration, and this path runs repeatedly when
the registry conn is half-open. Now an explicitly stopped timer. The
result channel was already buffered, so the worker goroutine can always
deposit and exit rather than parking on the send.
Regression coverage in zz_connadapter_deadline_test.go: the deadline fires
and reports Timeout(); a cleared deadline restores block-forever semantics;
an armed deadline does not break normal reads; an already-expired deadline
returns immediately. The first of these hangs for the full timeout and then
fails against the old stub.
NOTE: this is half the chain. Frame readers reach connAdapter through
runtime's streamAdapter, which did not expose SetReadDeadline at all, so the
type assertion that gates the idle teardown failed regardless of what the
daemon implemented. Fixed in pilot-protocol/runtime#31. web4 pins runtime
v0.3.1, so that must be released and bumped here before the timeout engages
in production.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
The bug that OOM-killed the fleet
connAdapter's deadline setters were stubs:They stored nothing and reported success. Anything setting a read deadline got no deadline and no error — a silent lie, worse than not implementing the method, because callers cannot detect the failure.
Readtherefore blocked on<-a.conn.RecvBufforever. A peer that opened a stream and went silent pinned its handler goroutine plus the wholeConnection: a 512-slotRecvBuf, 256-slotSendBuf, and three goroutine stacks — ~43 KiB each.The arithmetic matches the outage exactly
431 agents × that on a 256 GB host → system-wide kernel OOM on 2026-07-28. Agents reaped → restart-looped → stopped heartbeating → registry expired every registration → entire fleet unresolvable.
The daemon wasn't leaking past a bound; it was filling its documented bound and sitting there — which is why growth was ~100× and then plateaued. Idle daemons on other hosts were fine at 17 MB after 24 weeks, which is what identified this as per-request retention rather than a time-based leak.
Changes
SetReadDeadlinestores an absolute deadline (atomic — it may be set from a goroutine other than the one parked inRead), andReadselects onRecvBufagainst a timer, returning anet.ErrorwithTimeout() == true.SetDeadlinedelegates toSetReadDeadlineinstead of silently doing nothing.SetWriteDeadlinestays a no-op but is now documented as one —Writealready bounds itself viaconnAdapterWriteDeadline.withRegistryDeadlineno longer leaks a timer per call.time.Afterpins its timer for the full 8s, and this path runs repeatedly when the registry conn is half-open. Now an explicitly stopped timer.Tests
zz_connadapter_deadline_test.gocovers: the deadline fires and reportsTimeout(); a cleared deadline restores block-forever semantics; an armed deadline doesn't break normal reads; an already-expired deadline returns immediately.The first hangs for the full timeout and then fails against the old stub.
Full
pkg/daemonsuite passes (23.5s).Frame readers reach
connAdapterthrough runtime'sstreamAdapter, which didn't exposeSetReadDeadlineat all, so the type assertion gating the idle teardown failed regardless of what the daemon implemented.Fixed in pilot-protocol/runtime#31 (with a compile-time assertion so it can't regress silently).
web4 pins
runtime v0.3.1— that must merge, release, and be bumped here before the timeout engages in production. Merging this PR alone is correct and safe but does not by itself stop the leak.Deliberately not included
ipcConn.portsgrows monotonically, but there is no unbind command — ports are released only on client disconnect, so aremovePortwould be dead code.DeliverInOrderphase-3 re-append skips theMaxOOOBufcheck, but net count is conserved on the normal path and I could not construct unbounded growth. Not touching packet-path code speculatively.Both are noted rather than "fixed" — they're asymmetries, not demonstrated leaks.
🤖 Generated with Claude Code