Skip to content

fix(registry/etcdv3): guard dataListener.interestedURL with RWMutex - #3545

Closed
Aias00 wants to merge 1 commit into
apache:developfrom
Aias00:fix-etcd-interestedurl-race
Closed

fix(registry/etcdv3): guard dataListener.interestedURL with RWMutex#3545
Aias00 wants to merge 1 commit into
apache:developfrom
Aias00:fix-etcd-interestedurl-race

Conversation

@Aias00

@Aias00 Aias00 commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

What

dataListener.interestedURL was a plain slice with no lock; AddInterestedURL appends from the DoSubscribe path while DataChange reads it (slices.ContainsFunc) on the etcd watch goroutine, racing the slice header.

Why

// registry/etcdv3/listener.go (before)
type dataListener struct {
    interestedURL []*common.URL   // no lock
    listener      config_center.ConfigurationListener
}
func (l *dataListener) AddInterestedURL(url *common.URL) {
    l.interestedURL = append(l.interestedURL, url)   // DoSubscribe path
}
func (l *dataListener) DataChange(eventType remoting.Event) bool {
    ...
    if slices.ContainsFunc(l.interestedURL, serviceURL.URLEqual) {  // etcd watch goroutine
        l.listener.Process(...)
    }
}

Two near-simultaneous DoSubscribe calls, or a subscribe concurrent with an incoming etcd watch event, race on the slice header → torn reads, missed event matching. go test -race flags it.

Fix

Add a sync.RWMutex to dataListener. AddInterestedURL takes the write lock; DataChange does the ContainsFunc check under RLock, releases it, then dispatches Process outside the lock.

Tests

Added Test_dataListener_ConcurrentAddAndDataChange: 100 rounds of concurrent AddInterestedURL vs DataChange, passing under -race. registry/etcdv3 passes under -race.

Fixes #3544

dataListener.interestedURL was a plain slice with no lock; AddInterestedURL
appends from the DoSubscribe path while DataChange reads it on the etcd watch
goroutine, racing the slice header. Add a sync.RWMutex: AddInterestedURL takes
the write lock; DataChange does the ContainsFunc check under RLock and
dispatches Process outside the lock.

Fixes apache#3544

Co-Authored-By: Claude <noreply@anthropic.com>
@sonarqubecloud

Copy link
Copy Markdown

@codecov-commenter

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 55.32%. Comparing base (60d1c2a) to head (86d0f7e).
⚠️ Report is 875 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #3545      +/-   ##
===========================================
+ Coverage    46.76%   55.32%   +8.55%     
===========================================
  Files          295      462     +167     
  Lines        17172    36002   +18830     
===========================================
+ Hits          8031    19918   +11887     
- Misses        8287    14527    +6240     
- Partials       854     1557     +703     

☔ 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.

@Aias00 Aias00 closed this Jul 27, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants