Skip to content

fix: cancellation/teardown hardening sweep#131

Merged
joaomdsg merged 1 commit into
mainfrom
fix/cancellation-teardown
Jun 11, 2026
Merged

fix: cancellation/teardown hardening sweep#131
joaomdsg merged 1 commit into
mainfrom
fix/cancellation-teardown

Conversation

@joaomdsg

Copy link
Copy Markdown
Member

Four related hardening items from an adversarial re-review of the shutdown/teardown path. Conservative throughout — a teardown that hangs is worse than the issues fixed, so every new wait is bounded and the cancel/close happens before the wait (no new deadlock).

Items

  1. S4 — wait for background goroutines on shutdown. New App.bgWG. The projector, consumer, changes/broadcast tailers and the three TTL sweepers each bgWG.Add(1) at spawn + defer bgWG.Done(). Shutdown waits on bgWG after backplaneCancel() + backplane.Close() + closing stopSweep (so the goroutines observe the stop and exit), and the wait is bounded by the shutdown ctx via a done-chan + select { case <-done: case <-ctx.Done(): }. A goroutine wedged on a backend that ignores cancellation cannot hang the drain past its deadline — a leaked goroutine is strictly better than a hung shutdown.

  2. S3 — prompt Subscribe teardown regardless of backend. The projector/consumer/changes/broadcast loops now select on the record channel and backplaneDone, so teardown is prompt even against a backend that doesn't close the channel on ctx cancel. Reconnect-on-transient-close semantics are unchanged (a real channel close still re-subscribes from the cursor).

  3. S10 — ctx-aware CAS backoff. casSleep now takes a context.Context and selects on ctx.Done(), so a contended StateApp/StateSess.Update mid-retry aborts the backoff the instant the backplane ctx is cancelled instead of adding up to ~10ms to the drain. Threaded app.backplaneCtx from both Update call sites.

  4. Theme-1 — surface swallowed backplane errors. WARN-log (greppable via: backplane … prefix) the previously dropped errors: backplane.Close in Shutdown; Subscribe/Head failure before a tailer goroutine returns; the broadcast Append error; the reconcile/applyChange LoadSnapshot errors in appval.go. Control flow unchanged; the ok=false non-error case still does not log.

Tested vs. couldn't

Covered by tests:

  • S10casSleep returns promptly when the ctx is already cancelled (internal test); existing within-ceiling bound test updated to the new signature.
  • S4Shutdown does not hang on a backend whose Subscribe channel never closes, and returns before its own deadline (proves the bounded wait + the S3 ctx.Done wake).
  • Theme-1 — a failing-Subscribe stub backplane surfaces the greppable WARN through a capturing Logger.
  • Existing backplane-context cancellation tests (TestShutdownCancelsInflight…, …WritePathContext) stay green.

Implemented but not directly unit-tested (asserted via the full -race suite + existing conformance/backplanetest, not a bespoke test): the per-loop backplaneDone wake on the consumer/changes/broadcast tailers in isolation, and each individual LoadSnapshot/Append/Close WARN site beyond the projector-subscribe one. These are log-only / control-flow-preserving changes verified by the existing suite staying green.

No deadlock + bounded shutdown

backplaneCancel() + Close() + close(stopSweep) all run before bgWG.Wait(), so every tracked goroutine can observe its stop signal first; the wait itself is wrapped in a ctx-bounded select. Verified Shutdown(context.Background()) (no deadline) still completes because all goroutines exit on backplaneDone/stopSweep.

CI

./ci-check.sh green: gofmt, vet, golangci-lint, govulncheck, API-compat baselines, -race across all packages, and the allocation gates (CounterRender 183/185, Action 137/149, ActionWithLogger 137/149 — unchanged).

Four related hardening items from an adversarial re-review of the
shutdown/teardown path. Conservative throughout — a teardown that hangs
is worse than the issues fixed, so every new wait is bounded.

S4 — wait for background goroutines on shutdown:
Add App.bgWG. The projector, consumer, changes/broadcast tailers and the
three TTL sweepers each Add(1) at spawn + defer Done. Shutdown waits on
bgWG AFTER backplaneCancel + backplane.Close + closing stopSweep (so the
goroutines can observe the stop and exit first), and the wait is bounded
by the shutdown ctx: a goroutine wedged on a backend that ignores
cancellation cannot hang the drain past its deadline.

S3 — prompt Subscribe teardown regardless of backend:
The projector/consumer/changes/broadcast loops now select on the record
channel AND backplaneDone, so teardown is prompt even against a backend
that does not close the channel on ctx cancel. Reconnect-on-transient-
close semantics are unchanged (a real channel close still re-subscribes).

S10 — ctx-aware CAS backoff:
casSleep now takes a context and selects on ctx.Done, so a contended
StateApp/StateSess.Update mid-retry aborts the backoff the instant the
backplane context is cancelled instead of adding up to the cap to the
drain. Threaded app.backplaneCtx from both Update call sites.

Theme-1 — surface swallowed backplane errors:
WARN-log (greppable "via: backplane …" prefix) the previously dropped
errors: backplane.Close in Shutdown; Subscribe/Head failure before a
tailer goroutine returns; the broadcast Append error; the reconcile/
applyChange LoadSnapshot errors. Control flow is unchanged — the ok=false
non-error case still does not log. Operators now see the cause of state
divergence instead of only the symptom.

Tested: casSleep prompt-return on cancelled ctx (internal test); Shutdown
does not hang on a non-closing Subscribe channel and returns before its
deadline; a failing-Subscribe stub surfaces the theme-1 WARN. Existing
backplane-context cancellation tests and the full -race suite stay green.
@joaomdsg
joaomdsg merged commit e016303 into main Jun 11, 2026
4 checks passed
@joaomdsg
joaomdsg deleted the fix/cancellation-teardown branch June 11, 2026 15:12
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