xxs588 commented on PR #3269:
URL: https://github.com/apache/dubbo-go/pull/3269#issuecomment-4149690211
> ## Code Review: fix(race): close remaining #3247 shared-state races
> ### [P0] 阻断级问题
>
> **1. `registry/directory/directory.go:586-620` - 并发修复不完整**
>
> 新增的 `snapshotCacheInvokers()` 方法使用 RLock+copy 模式正确，但在 `toGroupInvokers()`
方法中仍直接访问 `dir.cacheInvokers`：
>
> ```go
> func (dir *RegistryDirectory) toGroupInvokers() []protocolbase.Invoker {
> groupInvokersMap := make(map[string][]protocolbase.Invoker)
> // 问题：这里直接访问 dir.cacheInvokers，未使用 snapshot 方法
> for _, ivk := range dir.cacheInvokers {
> // ...
> }
> }
> ```
>
> 需要统一修改为：
>
> ```go
> for _, ivk := range dir.snapshotCacheInvokers() {
> // ...
> }
> ```
>
> ### [P1] 规范级问题
>
> **1. `config_center/file/listener.go:45-80` - listenerSet 设计**
>
> 新增的 `listenerSet` 结构设计良好，使用 `Snapshot()` 返回切片避免迭代时修改。但文档说明不足：
>
> ```go
> // Snapshot returns a read-only listener snapshot for safe iteration.
> // Callers must not mutate listeners through this snapshot.
> ```
>
> 建议添加注释说明调用方不应修改返回的切片。
>
> **2. 测试辅助函数验证**
>
> `listener_test.go` 新增的 `drainEvents` 和 `assertNoEvent`
函数未单独测试，建议添加单元测试验证其正确性。
>
> 总体评价：竞态修复方向正确，但需要检查所有直接访问共享状态的地方是否已统一使用 snapshot 方法。
关于 toGroupInvokers
我这边重新对了当前分支代码，这里现在是走 cacheInvokersMap.Range(...) 做分组，不是直接遍历
dir.cacheInvokers。cacheInvokers 目前只在加锁的 helper（snapshotCacheInvokers /
swapCacheInvokers）里访问 然后剩下两个部分已补充注释和添加了相关单测
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
