Debugging a WebSocket hub that froze under load

I work on a jobs and networking marketplace. Under load, its real-time side started to misbehave. Chat would stall. New connections hung in a "loading" state. Users who had closed their tabs still showed as online.

Restarting the server cleared it up. That's the worst kind of symptom. It gives you a workaround, but no explanation.

This post covers how I tracked down the cause, how I reproduced it on purpose and what the fix looked like. It also covers what I got wrong, and who shipped the final version. I diagnosed the bug, wrote the stress test and prototyped the fix on a branch. The fix that was merged to the main branch was written by a teammate, based on the same ideas. I think the debugging is the more useful part to share anyway.

How the hub worked

Chat and notifications ran through a single in-process hub. A hub is one central piece that keeps track of every open socket and routes messages between them. It's a common pattern for Go WebSocket servers:

• Each socket is a Client. It has a buffered send chan []byte (capacity 256), a ReadPump goroutine that reads from the socket and a WritePump goroutine that writes to it.
• One Hub.Run() goroutine receives messages on a broadcast channel and fans them out to the recipients.
• The hub tracks connections in activeClients map[userID]map[*Client]bool. One user can have several sockets (two tabs, a phone), so each user maps to a set of clients. One sync.Mutex guards it.

The key thing about this design: Hub.Run() is a single goroutine, and every message passes through it. If it slows down, everything slows down. If it panics, chat is dead until the process restarts.

The symptoms

• The real-time API stalled under load, and messages stopped arriving.
• New sockets hung in a loading state instead of connecting.
• Ghost sessions: users stayed "online" after they disconnected.

Root cause 1: a map assignment that copied a reference

The most interesting bug was in how direct messages picked their recipients. A DM should go to all of the sender's sockets, so their other tabs stay in sync. It should also go to all of the receiver's sockets. The code built that set like this:

case RealtimeDirectMessage:
    senderClients, senderOk := h.activeClients[sender]
    // ...
    clientsInRoom = senderClients // alias, not a copy
    if receiverClients, receiverOk := h.activeClients[receiver]; receiverOk {
        for client := range receiverClients {
            clientsInRoom[client] = true // writes B's sockets into A's global entry
        }
    }

In Go, a map value is a reference to a hash table. Assigning it to another variable doesn't copy the table. Both variables point at the same one. So clientsInRoom = senderClients didn't make a temporary list of recipients. It made a second name for user A's entry in the global registry. Then the loop wrote user B's sockets into it.

After just one DM, user A's entry in activeClients held user B's sockets for good. That set off a chain of problems:

1. 1.Messages went to the wrong people. Any later message for A was also sent to B's sockets.
2. 2.Ghost sessions. When B disconnected, cleanup removed B's client from activeClients[B]. But it stayed in A's set, so the hub still thought it was connected.
3. 3.A slow fuse. Nothing reads from a dead client's send channel, so it slowly filled up.
4. 4.A panic. When a send found the channel full, the notification path closed it but left the client in the map:

default:
    log.Printf("Notification send channel full. Cleaning up client: %s", client.userID)
    close(client.send) // the client is still in the map

The client was still registered, so the next message to it tried to send on a closed channel. In Go, sending on a closed channel panics. So does closing it a second time. This happened inside the single hub goroutine, which was started with go hub.Run() and no recover. Once it panicked, no more messages went out.

Root causes 2 and 3: every reader waiting on one goroutine

The shared map explained the ghost sessions and the eventual crash. But it didn't fully explain why everything got slow under load before anything crashed. Two other things contributed.

An unbuffered fan-in. Fan-in means many senders feeding one receiver. The broadcast channel was created with make(chan *SocketResponsePayload), which has no buffer. Every ReadPump ended by handing its message to the hub:

c.hub.broadcast <- &persistedMsg

On an unbuffered channel, a send waits until the receiver takes the value. With one receiver and thousands of senders, every connection's read loop waits in line for Hub.Run(). That loop also held the mutex while it marshalled JSON and wrote logs, so each turn took longer.

Lock contention on connect. RegisterClient ran in the HTTP upgrade goroutine, and it took the hub mutex there to change the map. So it fought for the lock with Run, with presence checks like IsUserOnline and with every other connection being set up. Unregistration also happened in two places: directly, and through a non-blocking send on a channel. With two paths, nobody clearly owned the client's lifecycle.

Reproducing it on purpose

Concurrency bugs that only show up under load are hard to fix with confidence. You can't tell whether a change helped or you just got lucky. So before touching the hub, I wanted a way to trigger the problem every time.

First, I faked a slow consumer, so the "channel fills up" path would happen quickly. I committed this to a debug branch and reverted it the same evening:

send: make(chan []byte, 50), // smaller buffer, fills faster
// ...
case message, ok := <-c.send:
    // BUG TO TEST: simulate a client that can't keep up
    time.Sleep(5 * time.Second)

Then I wrote a load generator in Go. It opens a lot of real WebSocket connections, sends direct messages between random users and disconnects them again:

const (
    totalConnections = 10000
    spawnRate        = 5 * time.Millisecond
)

// for each connection:
c, _, err := websocket.DefaultDialer.Dial(wsURL, nil)
if err != nil {
    return
}
atomic.AddInt32(&connectedCount, 1)

go func() { // drain incoming frames
    for {
        if _, _, err := c.ReadMessage(); err != nil {
            return
        }
    }
}()

req := SocketRequest{Type: "direct", RecipientID: randomRecipient, Content: "Stress Test Payload"}
jsonMsg, _ := json.Marshal(req)
c.WriteMessage(websocket.TextMessage, jsonMsg)

holdTime := time.Duration(10+rand.Intn(20)) * time.Second
time.Sleep(holdTime) // then disconnect, exercising the unregister path

The random hold time matters. Disconnects are where the ghost sessions and the double-close came from. So the test had to keep opening and closing connections, not just open them. I removed the auth token from this snippet. A test like this should read credentials from the environment, never from the source code.

The fix I prototyped

The changes followed one idea. The hub goroutine should be the only owner of client state. And temporary data should never point at shared data.

Build a fresh set for each fan-out.

// Build a new set so fan-out can never modify the global registry
// ("map pollution": accidentally adding user B's client to user A's list).
clientsInRoom = make(map[*Client]bool)
if senderClients, ok := h.activeClients[sender]; ok {
    for client := range senderClients {
        clientsInRoom[client] = true
    }
}
if receiverClients, ok := h.activeClients[receiver]; ok {
    for client := range receiverClients {
        clientsInRoom[client] = true
    }
}

Remove and close in one place. Unregistration moved into Run. Removing the client from the map and closing its channel now happen together, under the lock, and only if the client is still there:

case client := <-h.unregister:
    h.mutex.Lock()
    if userClients, ok := h.activeClients[client.userID]; ok {
        if _, exists := userClients[client]; exists {
            delete(userClients, client)
            if len(userClients) == 0 {
                delete(h.activeClients, client.userID)
            }
            close(client.send)
        }
    }
    h.mutex.Unlock()

The "is it still there?" check and the close happen in the same locked section. And only this goroutine does either. So a channel can't be closed twice or used after it's closed. Also, when a send to a client fails, that client is now removed from the global map too, not just from the temporary set.

Buffer the fan-in. broadcast became make(chan *SocketResponsePayload, 256). Now readers can hand off a message and go back to reading, without waiting for the hub to finish its current turn.

Move registration off the request goroutine. The HTTP upgrade handler no longer takes the lock. It hands registration to Run, which updates the map under a short lock.

What I got wrong, and what actually shipped

My prototype had a real flaw. I didn't want to block the upgrade handler, so I sent on the register channel with a non-blocking select:

select {
case h.register <- client:
default:
    log.Println("Register channel full. Dropping client")
}

That doesn't block. But under exactly the load I was testing for, it can quietly drop a registration. The socket is open, but the hub doesn't know about it, so that user never gets messages. I did the same thing for unregistration. Swapping a visible stall for an invisible failure isn't a fix.

A teammate wrote the version that was merged to the main branch, a few days later. It used the same core ideas: copy the client set before fanning out, close channels safely and do cleanup outside the locked section. Some months later, the same teammate made registration synchronous. That change came with a buffered broadcast channel and a read pump that enqueues first. It's the right answer to my dropped-registration problem. Registration is rare and cheap, so it can afford to wait.

Takeaways

• Assigning a Go map copies a reference. Make a copy before you merge sets.
• Give each piece of shared state exactly one owner goroutine.
• Removing a client and closing its channel must happen together, in one place.
• Never let N producers do a blocking send into one consumer without a buffer or backpressure.
• A long-lived goroutine that everything depends on needs a recover, or at least a supervisor that restarts it loudly.
• Don't swap a blocking send for one that drops messages, unless dropping is really acceptable.
• Reproduce concurrency bugs on purpose: add artificial delay, then load-test with realistic connects and disconnects.
• Keep load-test results next to the change, so the improvement is measured, not assumed.