Skip to content

Commit 6f3a05a

Browse files
committed
Fix concurrent write panic in WebSocket connections
- Added per-connection write mutex to serialize websocket writes - Introduced clientWrapper struct with thread-safe writeJSON() method - Prevents race conditions when multiple goroutines write to same connection - Fixes panic: 'concurrent write to websocket connection' Fixes #XXX
1 parent cf9e2ed commit 6f3a05a

2 files changed

Lines changed: 55 additions & 24 deletions

File tree

CHANGELOG.md

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
## [1.5.2] - 2025-01-12
11+
12+
### Fixed
13+
- Fixed concurrent write panic in WebSocket connections: "concurrent write to websocket connection"
14+
- Added per-connection write mutex to serialize websocket writes and prevent race conditions
15+
- WebSocket connections now thread-safe for both cache updates and log streaming
16+
17+
### Technical Details
18+
- Introduced `clientWrapper` struct with mutex protection for each websocket connection
19+
- All websocket writes now go through thread-safe `writeJSON()` method
20+
- Prevents panic when multiple goroutines attempt to write to the same connection simultaneously
21+
- Affects both `/ws` (cache updates) and `/ws/logs` (log streaming) endpoints
22+
1023
## [1.5.1] - 2025-01-12
1124

1225
### Fixed

cmd/cache_server.go

Lines changed: 42 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -25,19 +25,32 @@ var (
2525
}
2626

2727
// WebSocket client manager
28-
clients = make(map[*websocket.Conn]bool)
28+
clients = make(map[*websocket.Conn]*clientWrapper)
2929
clientsMu sync.RWMutex
3030
broadcast = make(chan interface{}, 100)
3131

3232
// Log streaming clients
33-
logClients = make(map[*websocket.Conn]bool)
33+
logClients = make(map[*websocket.Conn]*clientWrapper)
3434
logClientsMu sync.RWMutex
3535
logBroadcast = make(chan LogMessage, 1000)
3636

3737
// Ensure background goroutines are started only once
3838
startOnce sync.Once
3939
)
4040

41+
// clientWrapper wraps a websocket connection with a write mutex to ensure thread-safe writes
42+
type clientWrapper struct {
43+
conn *websocket.Conn
44+
mu sync.Mutex
45+
}
46+
47+
// writeJSON safely writes JSON to the websocket connection with mutex protection
48+
func (cw *clientWrapper) writeJSON(v interface{}) error {
49+
cw.mu.Lock()
50+
defer cw.mu.Unlock()
51+
return cw.conn.WriteJSON(v)
52+
}
53+
4154
var cacheServerCmd = &cobra.Command{
4255
Use: "viewer",
4356
Short: "Start a web server to view cache data",
@@ -121,11 +134,11 @@ func logBroadcastManager() {
121134
logClientsMu.RLock()
122135
clientCount := len(logClients)
123136
var failedClients []*websocket.Conn
124-
for client := range logClients {
125-
err := client.WriteJSON(logMsg)
137+
for conn, wrapper := range logClients {
138+
err := wrapper.writeJSON(logMsg)
126139
if err != nil {
127140
log.Printf("DEBUG: Failed to send log to client: %v", err)
128-
failedClients = append(failedClients, client)
141+
failedClients = append(failedClients, conn)
129142
}
130143
}
131144
logClientsMu.RUnlock()
@@ -138,9 +151,11 @@ func logBroadcastManager() {
138151
// Clean up failed clients
139152
if len(failedClients) > 0 {
140153
logClientsMu.Lock()
141-
for _, client := range failedClients {
142-
delete(logClients, client)
143-
client.Close()
154+
for _, conn := range failedClients {
155+
if wrapper, exists := logClients[conn]; exists {
156+
wrapper.conn.Close()
157+
delete(logClients, conn)
158+
}
144159
}
145160
logClientsMu.Unlock()
146161
}
@@ -161,9 +176,9 @@ func handleLogsWebSocket(w http.ResponseWriter, r *http.Request) {
161176

162177
log.Printf("Logs WebSocket connection established from %s", r.RemoteAddr)
163178

164-
// Register log client
179+
// Register log client with wrapper
165180
logClientsMu.Lock()
166-
logClients[conn] = true
181+
logClients[conn] = &clientWrapper{conn: conn}
167182
logClientsMu.Unlock()
168183

169184
// Send a test log message to verify the connection works
@@ -422,14 +437,15 @@ func handleWebSocket(w http.ResponseWriter, r *http.Request) {
422437
}
423438
defer conn.Close()
424439

425-
// Register client
440+
// Register client with wrapper
441+
wrapper := &clientWrapper{conn: conn}
426442
clientsMu.Lock()
427-
clients[conn] = true
443+
clients[conn] = wrapper
428444
clientsMu.Unlock()
429445

430446
// Send initial data
431-
sendCacheData(conn)
432-
sendStatusData(conn)
447+
sendCacheData(wrapper)
448+
sendStatusData(wrapper)
433449

434450
// Clean up on disconnect
435451
defer func() {
@@ -454,20 +470,22 @@ func broadcastManager() {
454470
clientsMu.RLock()
455471
// Collect failed clients while holding read lock
456472
var failedClients []*websocket.Conn
457-
for client := range clients {
458-
err := client.WriteJSON(msg)
473+
for conn, wrapper := range clients {
474+
err := wrapper.writeJSON(msg)
459475
if err != nil {
460-
failedClients = append(failedClients, client)
476+
failedClients = append(failedClients, conn)
461477
}
462478
}
463479
clientsMu.RUnlock()
464480

465481
// Clean up failed clients with write lock
466482
if len(failedClients) > 0 {
467483
clientsMu.Lock()
468-
for _, client := range failedClients {
469-
delete(clients, client)
470-
client.Close()
484+
for _, conn := range failedClients {
485+
if wrapper, exists := clients[conn]; exists {
486+
wrapper.conn.Close()
487+
delete(clients, conn)
488+
}
471489
}
472490
clientsMu.Unlock()
473491
}
@@ -617,17 +635,17 @@ func broadcastStatusUpdate() {
617635
}
618636
}
619637

620-
func sendCacheData(conn *websocket.Conn) {
638+
func sendCacheData(wrapper *clientWrapper) {
621639
cacheData := getCacheDataForWS()
622-
_ = conn.WriteJSON(WSMessage{
640+
_ = wrapper.writeJSON(WSMessage{
623641
Type: "cache",
624642
Data: cacheData,
625643
})
626644
}
627645

628-
func sendStatusData(conn *websocket.Conn) {
646+
func sendStatusData(wrapper *clientWrapper) {
629647
statusData := getStatusDataForWS()
630-
_ = conn.WriteJSON(WSMessage{
648+
_ = wrapper.writeJSON(WSMessage{
631649
Type: "status",
632650
Data: statusData,
633651
})

0 commit comments

Comments
 (0)