Bug
BroadcastAll and BroadcastToWorkflow in pkg/public/ws/ws_hub.go hold the hub's read lock and then evict clients from inside the delivery loop:
func (h *Hub) BroadcastToWorkflow(workflowID string, message []byte) {
h.mutex.RLock()
defer h.mutex.RUnlock()
if clients, ok := h.workflowSubscriptions[workflowID]; ok {
for client := range clients {
select {
case client.send <- message:
default:
// If the client's buffer is full, assume it's gone and unregister it
h.unregisterClient(client) // <- takes h.mutex.Lock()
}
}
}
}
unregisterClient takes h.mutex.Lock() (ws_hub.go:89). sync.RWMutex is not reentrant, so the broadcasting goroutine blocks on itself forever — while still holding the read lock. Every later register, unregister and broadcast then queues behind it. The hub never recovers.
Trigger
Any client whose send channel (cap 4096, ws_hub.go:155) fills up:
Impact
All six event distributors fan out through this hub (pkg/workers/eventdistributer/{canvas,run,execution,event_created,queue_item,agent_session}.go), so one stalled client permanently freezes canvas updates, run and execution status, and agent sessions for every user connected to that pod, until the process restarts.
Nothing is logged when it happens. It presents to users as "the UI stopped updating" — and reloading doesn't help, because the hub itself is wedged, not the connection.
Reproduction
Registering a client that isn't draining its send channel and broadcasting to its workflow is enough. Goroutine dump from a test that does exactly that:
goroutine 11 [sync.RWMutex.Lock]:
sync.runtime_SemacquireRWMutex(...)
sync.(*RWMutex).Lock(...)
/usr/local/go/src/sync/rwmutex.go:155 +0x6b
ws.(*Hub).unregisterClient(0x2efe41630c80, 0x2efe416a14d0)
pkg/public/ws/ws_hub.go:89 +0x3e
ws.(*Hub).BroadcastToWorkflow(0x2efe41630c80, ...)
pkg/public/ws/ws_hub.go:138 +0x19b
Expected behaviour
A client that stops reading gets dropped. It must not take the hub down with it.
Suggested fix
Collect the stalled clients while holding the read lock, and unregister them after releasing it, so the write lock is only ever taken with no read lock held. Eviction behaviour stays the same and becomes visible in the logs.
I have a patch with tests and will open a PR against this issue.
Related, not covered here
handleMessage (ws_hub.go:249) logs every inbound client message at Info level, with a 1 MB read limit — an unbounded log-flood vector from a single connection. Happy to send that as a separate PR if you'd like it fixed.
Bug
BroadcastAllandBroadcastToWorkflowinpkg/public/ws/ws_hub.gohold the hub's read lock and then evict clients from inside the delivery loop:unregisterClienttakesh.mutex.Lock()(ws_hub.go:89).sync.RWMutexis not reentrant, so the broadcasting goroutine blocks on itself forever — while still holding the read lock. Every laterregister,unregisterand broadcast then queues behind it. The hub never recovers.Trigger
Any client whose
sendchannel (cap 4096, ws_hub.go:155) fills up:writePumpblocks up towriteWait(10s) per message while the buffer keeps filling.Impact
All six event distributors fan out through this hub (
pkg/workers/eventdistributer/{canvas,run,execution,event_created,queue_item,agent_session}.go), so one stalled client permanently freezes canvas updates, run and execution status, and agent sessions for every user connected to that pod, until the process restarts.Nothing is logged when it happens. It presents to users as "the UI stopped updating" — and reloading doesn't help, because the hub itself is wedged, not the connection.
Reproduction
Registering a client that isn't draining its
sendchannel and broadcasting to its workflow is enough. Goroutine dump from a test that does exactly that:Expected behaviour
A client that stops reading gets dropped. It must not take the hub down with it.
Suggested fix
Collect the stalled clients while holding the read lock, and unregister them after releasing it, so the write lock is only ever taken with no read lock held. Eviction behaviour stays the same and becomes visible in the logs.
I have a patch with tests and will open a PR against this issue.
Related, not covered here
handleMessage(ws_hub.go:249) logs every inbound client message atInfolevel, with a 1 MB read limit — an unbounded log-flood vector from a single connection. Happy to send that as a separate PR if you'd like it fixed.