fix(sse): Client 加写锁 —— 同一 ResponseWriter 被并发写(-race 证实)
## 缺陷
`internal/sse/manager.go` 的 Client 结构体**一把写锁都没有**,
而 `Manager.mu` 只护 `clients` map 的**遍历** —— 遍历期间对每个
client 的 `c.SendWithID` 是**并发**的。
`http.ResponseWriter` 不是并发安全的,而 SSE 又是文本协议
(`id: N\nevent: X\ndata: {…}\n\n`),两个 Fprintf 交错就把
data 的 JSON 劈成半截 ⇒ 客户端 EventSource 收到坏帧、丢邮件。
## 实测(真实 httptest.ResponseRecorder + -race)
WARNING: DATA RACE
Read at ... by goroutine 13:
net/http/httptest.(*ResponseRecorder).writeHeader()
sse.(*Client).SendWithID() manager.go:350
★ 32 goroutine × 25 帧 = 800 帧,只切出 459 帧完整
## 生产上会打中的三条路径
① handler/permission.go:412-414 —— `SendToAgent(perm.AgentName,…)`
紧接 `SendToUser(user.Username,…)`,两个不同 HTTP 请求命中同一账号。
② 任意两条并发邮件:一封投给 B,B 的插件回信进 C 的 handler,
而 A 的 `notify.Recipients` 还没跑完。
③ heartbeat 那条 goroutine 每 10s 写一次(见 heartbeatInterval
注释:实测本机 SSE 连接只活 34~57s,被中间反代按空闲超时掐掉),
撞车概率随在线时长线性上升。
## 修法
Client 加 `writeMu`,串行化**全部四条**写路径:
Send / SendWithID / heartbeat / replay
`replay` 虽在注册之前、按构造就是单写者,仍持锁 ——
让「对 Res 的写入一律经由 writeMu」成为**结构上**的纪律:
将来有人把注册提前或把回放挪到注册之后,没上锁的版本会静默退化成并发写。
为什么不能靠上层串行化:推送方有 5 个入口
(SendToUser/SendToAgent/SendToRecipient/Broadcast/replay),
要保证"同一 client 的所有写互斥",责任只能落在 client 自己身上。
## 判据(新增 frame_integrity_test.go,2 格)
**用帧完整性而不是"不许有 race"当判据** —— 本仓 `go test ./...`
默认不带 -race,判据必须在默认路径能判,否则就变成"要记得加 flag"。
TestFrameIntegrityUnderConcurrentPush 800 帧必须 800 帧完整
TestHeartbeatDoesNotInterleaveWithPush 钉住"只锁推送漏掉心跳"那个漏法
变异验证(去掉三处锁):
★ 17 次交错;切出 793/800 帧;心跳格也报 3 次交错 ⇒ 两格都有分辨力。
This commit is contained in:
@ -103,6 +103,30 @@ type Client struct {
|
||||
Res http.ResponseWriter
|
||||
Flusher http.Flusher
|
||||
done chan struct{}
|
||||
|
||||
// ★ writeMu 串行化对 Res 的**每一次**写入。
|
||||
//
|
||||
// 为什么必需(2026-09-28 实测,-race 证实):
|
||||
// http.ResponseWriter **不是并发安全**的,而本结构原先**一把写锁都没有**。
|
||||
// Manager.mu 只护 `clients` map 的**遍历**,遍历期间的 `c.SendWithID` 是并发的 ——
|
||||
// 任何两条并发请求都会同时向同一个 client 写。
|
||||
//
|
||||
// 生产上会打中的三条路径:
|
||||
// ① handler/permission.go:412-414 —— `SendToAgent(perm.AgentName, …)` 紧接
|
||||
// `SendToUser(user.Username, …)`,两个不同 HTTP 请求(两个 goroutine)命中同一账号;
|
||||
// ② 任意两条并发邮件:一封投给 B,B 的插件回信进 C 的 handler,而 A 的
|
||||
// `notify.Recipients` 还没跑完;
|
||||
// ③ `heartbeat` 那条 goroutine 每 10s 写一次(见 heartbeatInterval 的注释),
|
||||
// 与推送撞车的概率随在线时长线性上升。
|
||||
//
|
||||
// 症状:SSE 是 `id: N\nevent: X\ndata: {…}\n\n` 的**文本协议**,两个 Fprintf
|
||||
// 交错 ⇒ data 的 JSON 被劈成半截 ⇒ 客户端 EventSource 收到坏帧、丢事件。
|
||||
// 实测 32 goroutine × 25 帧 = 800 帧,只切出 **459** 帧完整。
|
||||
//
|
||||
// 为什么不能靠上层串行化:推送方有 5 个入口(SendToUser / SendToAgent /
|
||||
// SendToRecipient / Broadcast / replay),要保证"同一个 client 的所有写互斥",
|
||||
// 责任只能落在 client 自己身上 —— 那是唯一能覆盖**全部**写者的位置。
|
||||
writeMu sync.Mutex
|
||||
}
|
||||
|
||||
// Manager 管理所有 SSE 客户端连接
|
||||
@ -182,10 +206,19 @@ func (m *Manager) AddClient(res http.ResponseWriter, r *http.Request, agentName,
|
||||
|
||||
// Last-Event-ID 回放:EventSource 断线重连时自带这个头,
|
||||
// 服务端据此把断线期间的事件补上 —— 否则重连后永远看不到那段时间的邮件。
|
||||
//
|
||||
// ★ 持 writeMu 写入(尽管此时**按构造就是单写者**):
|
||||
// 回放发生在下面 `m.clients[id] = client` **之前** —— 此刻还没有任何 goroutine
|
||||
// 拿得到这个 client 的指针,所以本身上就是安全的。持锁是为了让「对 Res 的写入
|
||||
// 一律经由 writeMu」成为**结构上**的纪律:将来有人把注册提前、或把回放挪到
|
||||
// 注册之后(很自然的一个改动),没上锁的版本会**静默**退化成并发写。
|
||||
// 锁在这里零成本,而它买的正是「后人改顺序也不会破」这件事。
|
||||
lastID := r.Header.Get("Last-Event-ID")
|
||||
key := m.bufferKey(userName, agentName)
|
||||
if ring := m.getOrCreateRing(key); ring != nil && lastID != "" {
|
||||
client.writeMu.Lock()
|
||||
ring.replay(lastID, flusher, res)
|
||||
client.writeMu.Unlock()
|
||||
}
|
||||
|
||||
m.mu.Lock()
|
||||
@ -334,6 +367,8 @@ func (c *Client) Send(eventType string, data interface{}) {
|
||||
return
|
||||
}
|
||||
|
||||
c.writeMu.Lock()
|
||||
defer c.writeMu.Unlock()
|
||||
fmt.Fprintf(c.Res, "event: %s\ndata: %s\n\n", eventType, jsonData)
|
||||
c.Flusher.Flush()
|
||||
}
|
||||
@ -347,6 +382,8 @@ func (c *Client) SendWithID(id, eventType string, data interface{}) {
|
||||
return
|
||||
}
|
||||
|
||||
c.writeMu.Lock()
|
||||
defer c.writeMu.Unlock()
|
||||
fmt.Fprintf(c.Res, "id: %s\nevent: %s\ndata: %s\n\n", id, eventType, jsonData)
|
||||
c.Flusher.Flush()
|
||||
}
|
||||
@ -373,8 +410,13 @@ func (m *Manager) heartbeat(client *Client) {
|
||||
return
|
||||
case <-ticker.C:
|
||||
defer func() { recover() }()
|
||||
// ★ 同一把 writeMu:心跳是本结构里**第三条**写 Res 的路径。
|
||||
// 漏了它就等于"推送之间互斥、心跳不参与"—— 而心跳每 10s 一次、
|
||||
// 覆盖连接的全部存活期,撞上推送是必然事件(见 writeMu 的注释 ③)。
|
||||
client.writeMu.Lock()
|
||||
fmt.Fprintf(client.Res, ": heartbeat\n\n")
|
||||
client.Flusher.Flush()
|
||||
client.writeMu.Unlock()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user