Aias00 opened a new issue, #3539:
URL: https://github.com/apache/dubbo-go/issues/3539
### Problem
Two bugs in the Zookeeper config_center listener:
1. **`RemoveListener` uses a different key than `AddListener` — listener
leak.** `AddListener` qualifies the key (`namespace/key` → `buildPath(rootPath,
...)`) before storing under the qualified key, but `RemoveListener` passes the
**raw** caller key, so `CacheListener.RemoveListener` does
`keyListeners.Load(rawKey)` which never matches the stored qualified key →
`loaded` is false → the listener is never deleted. Every remove is a silent
no-op; stale listeners accumulate and keep receiving `DataChange` after
unsubscribe.
```go
// config_center/zookeeper/impl.go
func (c *zookeeperDynamicConfiguration) AddListener(key string, listener
..., ...) {
key =
strings.Join([]string{c.GetURL().GetParam(constant.ConfigNamespaceKey, ...),
key}, "/")
qualifiedKey := buildPath(c.rootPath, key)
c.cacheListener.AddListener(qualifiedKey, listener) // stored
qualified
}
func (c *zookeeperDynamicConfiguration) RemoveListener(key string,
listener ..., ...) {
c.cacheListener.RemoveListener(key, listener) // looked up
RAW -> no-op
}
```
2. **`CacheListener` inner listener map has no lock — fatal concurrent map
read+write.** `keyListeners` is a `sync.Map` whose **value** is a plain
`map[ConfigurationListener]struct{}` with no mutex (unlike
`metadata/report/zookeeper`'s locked `ListenerSet`).
`AddListener`/`RemoveListener` (router/config goroutines) write/delete the
inner map; `DataChange` (zk event goroutine) ranges it. Concurrent map
read+write is a fatal Go runtime error (`fatal error: concurrent map read and
map write`), not just a benign race.
### Current behavior
- Router `RemoveListener` calls (condition/script/affinity routers all call
`dynamicConfiguration.RemoveListener(key, ...)`) are no-ops → leaked stale
listeners keep firing `Process` against destroyed routers.
- A config push arriving while a router subscribes/unsubscribes crashes the
process with a fatal concurrent-map-access error.
### Expected behavior
1. `RemoveListener` must qualify the key identically to `AddListener` so the
stored entry is actually removed.
2. The inner listener set must be guarded by a mutex (or replaced with a
locked set), and `DataChange` must not hold that lock across the external
`Process` callback.
### Suggested approach
- Add a `qualifyKey(key)` helper used by **both** `AddListener` and
`RemoveListener` so the two keys cannot drift.
- Replace the inner `map[ConfigurationListener]struct{}` value with a
mutex-guarded `listenerSet` (`add`/`remove`/`snapshot`); `DataChange` snapshots
under the lock and dispatches `Process` outside the lock.
### Acceptance criteria
- [ ] `RemoveListener` removes the listener that `AddListener` stored (same
qualified key).
- [ ] The inner listener set is mutex-guarded; no fatal
concurrent-map-access under `-race`.
- [ ] `DataChange` dispatches `Process` outside the set lock.
- [ ] Regression tests: `qualifyKey` equality between Add/Remove;
`listenerSet` concurrency under `-race`.
--
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]
