client: Service.ctl has a shutdown race / Service.ctl 关闭竞态
中文说明
问题
client.Service.ctl 在关闭时由 stop() 持有 ctlMu 写锁清空,但 UpdateAllConfigurer 和控制器监控路径会在锁外读取该字段。并发关闭与配置更新时,Race Detector 能稳定报告同一字段的读写竞争;UpdateAllConfigurer 还存在判空与实际调用读取不同值的风险。
复现
回归测试并发执行 Service.UpdateAllConfigurer(nil, nil) 与 Service.stop(),在当前 dev 上运行:
go test -race -count=20 ./client -run '^TestUpdateAllConfigurerAndStopSynchronizeControl$'
修复前 Race Detector 栈指向 client/service.go 中 UpdateAllConfigurer 的读取与 stop 的写入。
建议修复
通过现有 ctlMu 获取短生命周期的 Control 快照,释放锁后再调用 Control 方法或等待 Done()。Run 将首次控制器快照传给监控协程,重连后重新获取快照;UpdateAllConfigurer 对同一快照完成判空和调用。
该修复不修改公开 API、配置格式、重连间隔、关闭顺序或 Control 内部并发模型,也不会在持有 ctlMu 时等待或调用 Control。
English Description
Bug
client.Service.ctl is cleared by stop() while holding the ctlMu write lock, but UpdateAllConfigurer and the controller-monitoring path read the field outside the lock. Concurrent shutdown and configuration updates reliably trigger the Race Detector on this field. UpdateAllConfigurer can also observe a different value between its nil check and method call.
Reproduction
A regression test runs Service.UpdateAllConfigurer(nil, nil) concurrently with Service.stop():
go test -race -count=20 ./client -run '^TestUpdateAllConfigurerAndStopSynchronizeControl$'
Before the fix, the Race Detector reports the read in UpdateAllConfigurer racing with the write in stop in client/service.go.
Proposed fix
Read Control through a short-lived snapshot protected by the existing ctlMu, then release the lock before invoking Control methods or waiting on Done(). Run passes its initial snapshot to the monitoring goroutine, reconnects take a fresh snapshot, and UpdateAllConfigurer uses one snapshot for both the nil check and call.
This does not change public APIs, configuration, retry timing, shutdown order, or Control internals. It never waits on or invokes Control while holding ctlMu.
Source: fatedier/frp