fix(ws): process register/unregister on one ordered channel

Register and Unregister travelled on two separate channels, and Run's
select picks randomly when both are ready: a fast connect/disconnect could
process the unregister first (a silent no-op for a not-yet-known client)
and then the register — admitting an already-dead connection as a ghost
client that held presence and swallowed broadcasts until the stale sweep
reaped it minutes later. One tagged event channel preserves each
connection's Register→Unregister submission order, making the inversion
structurally impossible. Found via TestHub_ConcurrentRegisterUnregister
failing the P1 gate under -race on windows-latest (2 ghosts after churn);
that test now settles in milliseconds instead of polling out its deadline.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
J3vb
2026-07-19 09:33:33 +02:00
co-authored by Claude Fable 5
parent 2eec831d6a
commit 0baccb58ee
+28 -15
View File
@@ -29,13 +29,18 @@ type broadcastMsg struct {
// Hub manages all active WebSocket clients and routes messages between them.
// All exported methods are safe to call from multiple goroutines.
type Hub struct {
clients map[int64]*Client
mu syncutil.RWMutex
db *db.DB
limiter *auth.RateLimiter
broadcast chan broadcastMsg
register chan *Client
unregister chan *Client
clients map[int64]*Client
mu syncutil.RWMutex
db *db.DB
limiter *auth.RateLimiter
broadcast chan broadcastMsg
// clientEvents carries register AND unregister requests on one channel so
// a connection's Register→Unregister sequence is processed in submission
// order. With two separate channels, Run's select picked randomly between
// them when both were ready — a fast connect/disconnect could process the
// unregister first (a no-op for an unknown client) and then the register,
// admitting an already-dead client as a ghost until the stale sweep.
clientEvents chan clientEvent
stop chan struct{}
stopOnce sync.Once
gracefulOnce sync.Once
@@ -94,8 +99,7 @@ func NewHub(database *db.DB, limiter *auth.RateLimiter, svc *service.Services) *
db: database,
limiter: limiter,
broadcast: make(chan broadcastMsg, 1024),
register: make(chan *Client, 32),
unregister: make(chan *Client, 32),
clientEvents: make(chan clientEvent, 64),
stop: make(chan struct{}),
pubsub: NewPubSub(),
topicLimiter: NewTopicRateLimiter(topicRateLimitPerSecond, time.Second),
@@ -263,10 +267,12 @@ func (h *Hub) Run() {
select {
case <-h.stop:
return
case c := <-h.register:
h.registerNow(c)
case c := <-h.unregister:
h.unregisterNow(c)
case ev := <-h.clientEvents:
if ev.add {
h.registerNow(ev.c)
} else {
h.unregisterNow(ev.c)
}
case bm := <-h.broadcast:
h.deliverBroadcast(bm)
case <-staleTicker.C:
@@ -381,12 +387,19 @@ func (h *Hub) GetClient(userID int64) *Client {
// Register queues a client for registration with the hub.
func (h *Hub) Register(c *Client) {
h.register <- c
h.clientEvents <- clientEvent{c: c, add: true}
}
// Unregister queues a client for removal from the hub.
func (h *Hub) Unregister(c *Client) {
h.unregister <- c
h.clientEvents <- clientEvent{c: c}
}
// clientEvent is a register (add=true) or unregister (add=false) request.
// Both kinds share one channel so per-connection ordering is preserved.
type clientEvent struct {
c *Client
add bool
}
func (h *Hub) registerNow(c *Client) {