From 0baccb58eede662934e362dbb5d63d49e5e09433 Mon Sep 17 00:00:00 2001
From: J3vb <192430104+J3vb@users.noreply.github.com>
Date: Sun, 19 Jul 2026 09:33:33 +0200
Subject: [PATCH] fix(ws): process register/unregister on one ordered channel
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 Server/ws/hub.go | 43 ++++++++++++++++++++++++++++---------------
 1 file changed, 28 insertions(+), 15 deletions(-)

diff --git a/Server/ws/hub.go b/Server/ws/hub.go
index 4710547a..43453778 100644
--- a/Server/ws/hub.go
+++ b/Server/ws/hub.go
@@ -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) {