fix(coderd): make chatd stream sync poller a callback fanout to fix send-on-closed-channel panic - #28746
Draft
ethanndickson wants to merge 1 commit into
Draft
fix(coderd): make chatd stream sync poller a callback fanout to fix send-on-closed-channel panic#28746ethanndickson wants to merge 1 commit into
ethanndickson wants to merge 1 commit into
Conversation
…end-on-closed-channel panic The stream sync poller delivered hints over per-subscriber channels that unregister closed under the poller mutex, while pollOnce sent on them from a lock-free snapshot. A subscriber unregistering mid-poll made pollOnce panic with "send on closed channel", crashing coderd. Any authenticated user could trigger this by opening and closing chat streams (GHSA-7x3x-59xg-4hrc). Convert the poller to a callback fanout, mirroring pubsub.Subscribe: Register now takes a deliver callback and returns only an unregister func, unregister is delete-only (nothing is ever closed), and the consumer funnels poller hints into its existing consumer-owned updateCh with the same streamCtx guard already used for pubsub hints. This makes the panic unrepresentable and also removes the nil-poller closed-channel return that instantly terminated streams via the ok-check. Adds a regression race test churning register/unregister against pollOnce; the previous implementation panics under it.
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.
Closes SEC-515
Problem
streamSyncPollerdelivered hints over per-subscriber channels.unregisterclosed a subscriber's channel under the poller mutex, whilepollOncesent on it from a lock-free snapshot taken before the DB query. A subscriber unregistering mid-poll madepollOncepanic withsend on closed channel, taking down the whole coderd process. Any authenticated user could trigger this by opening and closing chat streams (GHSA-7x3x-59xg-4hrc). On dev.coder.com this crashed 7 pods within 12 hours on 2026-08-27:Fix
Convert the poller to a callback fanout, the same shape as
pubsub.Subscribe/MemoryPubsub:Register(chatID, deliver func(streamSyncHint))returns only anunregister func().unregisteris delete-only; nothing is ever closed, so the panic is unrepresentable rather than guarded.updateCh, using the identicalstreamCtx.Done()guard already used for the pubsub producer ten lines above. ThepollerChselect arm is deleted.unregisterinstead of an already-closed channel, removing a latent bug where the consumer's!okcheck would instantly terminate every stream.Delivery stays outside the poller mutex (existing snapshot design), so a slow consumer cannot stall polling for other chats; the callback-may-race-unregister contract matches what the consumer already assumes for async
PGPubsubdelivery and is documented onRegister.Testing
stream_sync_poller_internal_test.go) churns register/unregister across 8 goroutines against 1000pollOncedeliveries; the previous implementation panics under it.go test -race ./coderd/x/chatd/...passes.