fix:cache manager race #3569

[xiaobaicai66695]

说明

Fixes #3569

本 PR 保证服务发现 CacheManager 在并发更新期间生成一致的缓存快照，并使定时落盘任务能够可靠、幂等地停止。

问题分析

进一步检查发现，项目通过 *lru.Cache 使用 github.com/hashicorp/golang-lru v0.5.4。该类型本身是并发安全的，其 Get、Add、Remove、Keys、Len 和 Purge 均有内部锁保护。

实际问题发生在复合操作层面：

1. GetAll 先通过 Keys() 获取键快照。
2. 另一个 goroutine 可能在此时删除其中一个键。
3. 后续 Get(key) 返回 (nil, false)。
4. 原实现忽略了布尔返回值，将 nil 写入结果。
5. dumpCache 调用 gob.Register(nil)，最终触发 panic。

因此，可复现的问题是缓存快照不具备原子性，并在定时落盘时引发崩溃，而不是底层 LRU 双向链表发生了无锁数据竞争。

原 StopDump 还会在持有 CacheManager.lock 时向无缓冲通道发送停止信号。如果落盘 goroutine 正在等待同一把锁，停止流程存在死锁窗口。

修改内容

• 使用同一把互斥锁保护所有 CacheManager 缓存操作。
• 保证 GetAll 相对于 Get、Set 和 Delete 是原子快照。
• 忽略已经不存在的键，避免将 nil 写入落盘数据。
• 缓存加载统一通过加锁后的 Set 方法执行。
• 文件写入前先生成一致的缓存快照。
• 使用 sync.Once 保证 StopDump 可以安全重复调用。
• 使用 done 通道等待落盘 goroutine 完全退出。
• goroutine 退出时停止 ticker 并关闭 done。
• 增加并发访问、落盘重载和并发停止回归测试。

回归测试设计

TestCacheManagerConcurrentAccess 启动 16 个 goroutine。每个 goroutine 执行 500 轮并发 Set、Get、Delete 和 GetAll，同时启用 1ms 周期的定时落盘任务。

每轮测试包含：

• 8,000 次 Set
• 8,000 次 Get
• 2,672 次 Delete
• 1,152 次 GetAll

并发操作结束后，测试还会：

• 等待定时落盘文件生成；
• 停止落盘 goroutine；
• 从落盘文件重新加载缓存；
• 验证缓存可正常解码、值不为 nil 且类型正确。

TestCacheManagerConcurrentStopDump 分别覆盖开启和关闭落盘两种模式，每种模式由 64 个 goroutine 同时调用 StopDump，并验证重复停止不会阻塞或死锁。

自测报告

测试环境：Go 1.25.5、Windows/AMD64、CGO_ENABLED=1、32 个逻辑处理器。

测试命令	结果
go test -count=1 ./registry/servicediscovery/store	通过，9.337s
go test -race -count=1 ./registry/servicediscovery/store	通过，10.810s
目标回归测试 -count=20	通过，2.239s
目标回归测试 -race -count=20	通过，4.412s
go test -count=1 ./registry/servicediscovery/...	全部通过
go vet ./registry/servicediscovery/...	通过，无诊断信息
store 包语句覆盖率	90.2%

20 轮 race 回归共执行 396,480 次缓存读写及快照操作，并执行 2,560 次并发 StopDump 调用。

覆盖率明细

• Get、Set、Delete、GetAll、StopDump、destroy：100%
• runDumpTask：90.9%
• getAllLocked：88.9%
• loadCache：84.6%
• dumpCache：83.3%
• NewCacheManager：83.3%
• 总语句覆盖率：90.2%

性能测试

32 线程环境下，每项执行 5 次 1 秒 benchmark，取中位数：

操作	修改前	修改后	变化
并行 Get	103.2 ns/op	121.0 ns/op	+17.2%
混合读写	114.2 ns/op	125.0 ns/op	+9.5%
并行 GetAll	16.179 μs/op	5.207 μs/op	-67.8%

由于底层缓存本身也有锁，外层互斥锁会增加单次操作开销；但原子 GetAll 的性能明显提升，内存分配由 10,520 B / 12 allocs 降至约 6,104 B / 5 allocs。

已知但不属于本 PR 的问题

• go test -race ./registry/servicediscovery/... 会在既有的 TestServiceDiscoveryRegistryUnRegister_Concurrent 中报告竞态。该测试明确无锁修改 sdReg.instances，在基线提交上同样失败，与本 PR 无关。store 范围的 race 测试全部通过。
• 显式调用 Set(key, nil) 仍可能在 gob.Register(nil) 处 panic。生产元数据路径会拒绝 nil，建议单独处理。
• 缓存文件仍直接写入最终路径，没有使用临时文件加原子重命名。本 PR 未修改该既有行为。

检查清单

• 确认目标分支为 develop
• 代码已通过本地测试
• 已添加能够证明修复有效的回归测试

[Copilot]

Pull request overview

This PR fixes a concurrency bug in the service-discovery CacheManager by making cache snapshotting (GetAll) consistent under concurrent updates and by making the periodic dump goroutine reliably stoppable (including safe repeated StopDump calls), addressing the panic scenario described in #3569.

Changes:

• Serialize CacheManager cache operations (Get/Set/Delete/GetAll) with a single mutex and implement GetAll as an atomic snapshot.
• Rework dump lifecycle control with stopOnce + done to make stopping idempotent and avoid deadlock windows.
• Add regression tests covering concurrent access with frequent dumps and high-concurrency StopDump calls.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
registry/servicediscovery/store/cache_manager.go	Adds locking around cache operations, makes GetAll snapshot-safe, and makes dump goroutine stop idempotent via sync.Once + done.
registry/servicediscovery/store/cache_manager_test.go	Adds concurrent-access + concurrent-stop regression tests to validate the race/deadlock fixes.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[codecov-commenter]

Codecov Report

❌ Patch coverage is 94.44444% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 54.60%. Comparing base (48d6e69) to head (799c8a2).

Files with missing lines	Patch %	Lines
registry/servicediscovery/store/cache_manager.go	94.44%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3600      +/-   ##
==========================================
+ Coverage   54.57%   54.60%   +0.02%     
==========================================
  Files         460      460              
  Lines       35443    35465      +22     
==========================================
+ Hits        19344    19364      +20     
- Misses      14543    14544       +1     
- Partials     1556     1557       +1     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.