Skip to content

fix: 使用 context 替代 closeChan 修复 processLoop 中 send on closed channel …#34

Open
caiwc wants to merge 2 commits into
open-dingtalk:mainfrom
easyops-cn:main
Open

fix: 使用 context 替代 closeChan 修复 processLoop 中 send on closed channel …#34
caiwc wants to merge 2 commits into
open-dingtalk:mainfrom
easyops-cn:main

Conversation

@caiwc
Copy link
Copy Markdown

@caiwc caiwc commented Mar 19, 2026

…panic

子 goroutine(读消息、ping 超时)在 processLoop 退出后仍可能向已关闭的
closeChan 发送数据,导致 panic: send on closed channel。

使用 context.WithCancel 替代 closeChan 作为关闭信号机制,所有子 goroutine 通过 cancelLoop() 通知退出,主循环监听 loopCtx.Done(),彻底消除竞态。

Fixes: #27, #28, #32

…panic

子 goroutine(读消息、ping 超时)在 processLoop 退出后仍可能向已关闭的
closeChan 发送数据,导致 panic: send on closed channel。

使用 context.WithCancel 替代 closeChan 作为关闭信号机制,所有子 goroutine
通过 cancelLoop() 通知退出,主循环监听 loopCtx.Done(),彻底消除竞态。

Fixes: open-dingtalk#27, open-dingtalk#28, open-dingtalk#32

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@ViolaXJS
Copy link
Copy Markdown

求合并,同遇到这个问题

@wqyenjoy
Copy link
Copy Markdown

AI review出来的小问题,需要麻烦看卡 @caiwc

仍存在的小瑕疵(建议作者顺手修)
defer func() {
cancelLoop()
close(pongChan) // ← 这一行其实可以删掉
}()
cancelLoop() 已经让 PongHandler 走 <-loopCtx.Done() 分支返回,没人再读 pongChan,不需要再关闭。
而保留 close(pongChan) 反而留了一个理论窗口:cancelLoop() 调用完到 close(pongChan) 之间,若 PongHandler 恰好被触发,select 的两个 case 都 ready,Go 的 select 是伪随机选择的,仍有概率走到 send 分支并 panic(虽然非常窄)。
删掉这一行后,pongChan 由 GC 自然回收,逻辑更干净、也彻底没有残留竞态。

@caiwc caiwc closed this May 25, 2026
@caiwc caiwc reopened this May 25, 2026
@caiwc
Copy link
Copy Markdown
Author

caiwc commented May 25, 2026

AI review出来的小问题,需要麻烦看卡 @caiwc

仍存在的小瑕疵(建议作者顺手修) defer func() { cancelLoop() close(pongChan) // ← 这一行其实可以删掉 }() cancelLoop() 已经让 PongHandler 走 <-loopCtx.Done() 分支返回,没人再读 pongChan,不需要再关闭。 而保留 close(pongChan) 反而留了一个理论窗口:cancelLoop() 调用完到 close(pongChan) 之间,若 PongHandler 恰好被触发,select 的两个 case 都 ready,Go 的 select 是伪随机选择的,仍有概率走到 send 分支并 panic(虽然非常窄)。 删掉这一行后,pongChan 由 GC 自然回收,逻辑更干净、也彻底没有残留竞态。

@wqyenjoy 已处理,删掉了 close(pongChan),这里确实不用关;同时补了个回归用例,覆盖 processLoop 退出后残留 PongHandler 被触发不 panic。提交在 c0fa0bc

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

长时间挂机会发生panic

3 participants