修复多处资源泄漏、错误处理缺陷与并发数据竞争#1693
Open
diaoyunxi wants to merge 1 commit into
Open
Conversation
本次提交修复代码审查中发现的多处缺陷: 资源泄漏: - token/encrypt.go: writeKey/fetchKey 未关闭文件句柄, 且 fetchKey 忽略 io.Copy 错误 - config/configuration.go: strict 模式二次打开配置文件未关闭, 导致 fd 泄漏 - cmd/cloudflared/updater/update.go: AutoUpdater.Run 的 ticker 未 Stop - features/selector.go: refreshLoop 的 ticker 未 Stop - hello/hello.go: sseHandler 的 ticker 未 Stop - cmd/cloudflared/updater/check.go: StartWarningCheck 使用无缓冲 channel, 网络请求完成前被非阻塞读取会导致 goroutine 永久阻塞泄漏 - cmd/cloudflared/tail/cmd.go: websocket.Dial 返回错误时未关闭 resp.Body 错误处理: - tunnelrpc/pogs/session_manager.go: RegisterUdpSession 忽略 SetTraceContext 错误, 与同函数其他字段处理不一致 - cmd/cloudflared/updater/workers_update.go: 下载二进制与生成批处理文件时忽略 Close 错误, 可能使用不完整文件执行更新 - cmd/cloudflared/linux_service.go: copyFile 忽略 Close 错误, 可能在刷盘失败时保留不完整的 service 配置 并发安全: - quic/v3/session.go: session.log 在 Migrate 中无锁写入, 与 readLoop/writeLoop/Write 并发读取产生数据竞争, 改用 atomic.Pointer 保护 逻辑修复: - datagramsession/session.go: sendFunc 失败时未关闭会话, 在 origin 持续上报数据时陷入只读只失败的快速循环, 改为关闭会话 (与 quic/v3 行为一致)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
背景
对 cloudflared 代码库进行系统性代码审查后, 发现多处资源泄漏、错误处理缺陷与并发数据竞争问题。本 PR 修复其中 12 处确信的真实缺陷, 均为最小化外科手术式修改, 未改变任何公共 API。
修改内容
资源泄漏
token/encrypt.gowriteKey/fetchKey未关闭文件句柄;fetchKey还忽略了io.Copy错误io.Copy错误config/configuration.godefer file.Close()cmd/cloudflared/updater/update.goAutoUpdater.Run的time.NewTicker未Stopdefer ticker.Stop()features/selector.gorefreshLoop的 ticker 未Stopdefer ticker.Stop()hello/hello.gosseHandler的 ticker 未Stopdefer ticker.Stop()cmd/cloudflared/updater/check.goStartWarningCheck使用无缓冲 channel,getWarning用select/default非阻塞读取, 若在网络请求完成前被读取, 发送方 goroutine 永久阻塞泄漏make(chan string, 1)cmd/cloudflared/tail/cmd.gowebsocket.Dial返回错误时未关闭resp.Body(nhooyr.io/websocket 要求调用方关闭)defer resp.Body.Close()错误处理
tunnelrpc/pogs/session_manager.goRegisterUdpSession中SetTraceContext错误被_ =吞掉, 与同函数其他字段处理不一致cmd/cloudflared/updater/workers_update.godownload与writeBatchFile忽略Close错误, 可能用不完整的二进制/批处理文件执行更新cmd/cloudflared/linux_service.gocopyFile忽略Close错误, 刷盘失败时仍保留不完整的 service 配置 (ok=true先于 Close 设置)并发安全
quic/v3/session.gosession.log(*zerolog.Logger) 在Migrate中无锁写入, 而readLoop/writeLoop/Write并发读取, 构成数据竞争 (go test -race会报DATA RACE)。s.eyeball已用atomic.Pointer保护, 唯独s.log遗漏atomic.Pointer[zerolog.Logger], 用Store/Load访问逻辑修复
datagramsession/session.godstToTransport中sendFunc失败时返回closeSession=false, 导致会话不关闭; 当 origin 持续上报数据时会陷入"读取-发送失败-日志-重复"的快速循环, 耗费 CPU 并刷爆日志return true, sendErr关闭会话, 与quic/v3session.readLoop 中发送失败即关闭的行为一致验证
make test lint; 改动均已人工逐行核对语法与并发语义, 恳请 CI 校验。备注
审查中还发现
ingress/icmp_linux.go存在一处 ICMP flow 的 goroutine+socket 泄漏 (listenResponse永久阻塞在ReadFrom, ctx 取消时不关闭 funnel)。因修复需在共享的packet.FunnelTracker上新增方法并处理双重关闭语义, 为控制本 PR 风险与范围暂未包含, 建议作为后续独立 PR 处理。