Aias00 opened a new issue, #3520:
URL: https://github.com/apache/dubbo-go/issues/3520
### Problem
`cluster/directory/static.directory` reads the `invokers` slice header (and
backing array) in `List` and `IsAvailable` **without any lock**, while
`Destroy` reassigns `dir.invokers = []protocolbase.Invoker{}` under
`base.Directory.mutex` (via `DoDestroy`). This is a data race on the slice
header (ptr/len/cap) per the Go memory model: a concurrent `Destroy` (provider
destroy cascade) and an in-flight `List`/`IsAvailable` (e.g. `available`
cluster invoker calls `List` before `CheckWhetherDestroyed`) trip `go test
-race`.
`base.Directory.mutex` is unexported, so the `static` package cannot acquire
it directly; the static directory needs its own lock for `invokers`.
### Current behavior
```go
// cluster/directory/static/directory.go
type directory struct {
*base.Directory
invokers []protocolbase.Invoker // no dedicated lock
}
func (dir *directory) IsAvailable() bool {
...
if len(dir.invokers) == 0 { ... } // unlocked read of slice header
for _, invoker := range dir.invokers { ... } // unlocked range
}
func (dir *directory) List(...) []protocolbase.Invoker {
l := len(dir.invokers) // unlocked read
invokers := make(...)
copy(invokers, dir.invokers) // unlocked read of header +
backing array
...
}
func (dir *directory) Destroy() {
dir.DoDestroy(func() {
for _, ivk := range dir.invokers { ivk.Destroy() }
dir.invokers = []protocolbase.Invoker{} // write under
base.Directory.mutex
})
}
```
### Expected behavior
`invokers` reads in `List`/`IsAvailable` and the write in `Destroy` should
be guarded by the same lock, so destroy never races an in-flight read.
### Suggested approach
- Add a `sync.RWMutex` to the static `directory` (since
`base.Directory.mutex` is not accessible cross-package).
- `IsAvailable`/`List` snapshot `invokers` under `RLock` and iterate the
snapshot outside the lock.
- `List` releases the lock before calling `RouterChain()` (which takes
`base.Directory.mutex`) so the two locks never nest and there is no deadlock.
- `Destroy`'s closure writes `invokers` under the new lock.
### Acceptance criteria
- [ ] `List`/`IsAvailable` read `invokers` under the directory lock.
- [ ] `Destroy` writes `invokers` under the same lock.
- [ ] A concurrency test (concurrent `List`/`IsAvailable` vs `Destroy`)
passes under `-race`.
- [ ] No deadlock between the directory lock and `base.Directory.mutex`.
--
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]
