You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by "Aias00 (via GitHub)" <gi...@apache.org> on 2026/06/17 08:03:53 UTC
[PR] fix(registry): add lock protection for serviceListeners map access [dubbo-go]
Aias00 opened a new pull request, #3442:
URL: https://github.com/apache/dubbo-go/pull/3442
## Problem
`serviceListeners` map in `serviceDiscoveryRegistry` was read/written without holding the lock, while the same struct's `serviceMappingListeners` already had proper lock protection via `s.lock`. This could cause data races when concurrent subscribe/unsubscribe operations access the map.
### Unprotected access locations
- `SubscribeURL` (line 365): read `s.serviceListeners[serviceNamesKey]` — no lock
- `SubscribeURL` (line 381): write `s.serviceListeners[serviceNamesKey] = listener` — no lock
- `UnSubscribe` (line 203): read `s.serviceListeners[serviceNamesKey]` — no lock
### Contrast with protected `serviceMappingListeners`
- `findMappedServices`: `s.lock.Lock()` / `s.lock.Unlock()`
- `stopListen`: `s.lock.Lock()` / `s.lock.Unlock()`
## Fix
- **`SubscribeURL`**: Use `s.lock.Lock()` / `s.lock.Unlock()` to protect the check-then-act read+write of `serviceListeners`. A write lock is required (not RLock) because the method follows a check-then-act pattern — it first reads to check if a listener exists, then conditionally creates and writes one. Using RLock would allow concurrent reads but not prevent the TOCTOU race.
- **`UnSubscribe`**: Use `s.lock.RLock()` / `s.lock.RUnlock()` for the read-only access of `serviceListeners`, consistent with read-lock usage elsewhere in the codebase.
## Testing
- `go build ./registry/servicediscovery/...` ✅
- `go vet ./registry/servicediscovery/...` ✅
- `go test ./registry/servicediscovery/... -count=1` ✅
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(registry): add lock protection for serviceListeners map access [dubbo-go]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3442:
URL: https://github.com/apache/dubbo-go/pull/3442#issuecomment-4736821850
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3442) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3442&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3442&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3442&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3442&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3442&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3442)
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(registry): add lock protection for serviceListeners map access [dubbo-go]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3442:
URL: https://github.com/apache/dubbo-go/pull/3442#issuecomment-4738073946
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3442) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3442&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3442&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3442&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3442&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3442&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3442)
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(registry): add lock protection for serviceListeners map access [dubbo-go]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3442:
URL: https://github.com/apache/dubbo-go/pull/3442#issuecomment-4727509860
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3442) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3442&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3442&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3442&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3442&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3442&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3442)
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(registry): add lock protection for serviceListeners map access [dubbo-go]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3442:
URL: https://github.com/apache/dubbo-go/pull/3442#issuecomment-5078963533
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3442) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3442&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3442&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3442&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3442&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3442&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3442)
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(registry): add lock protection for serviceListeners map access [dubbo-go]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3442:
URL: https://github.com/apache/dubbo-go/pull/3442#issuecomment-4815447566
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3442) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3442&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3442&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3442&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3442&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3442&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3442)
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(registry): add lock protection for serviceListeners map access [dubbo-go]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3442:
URL: https://github.com/apache/dubbo-go/pull/3442#issuecomment-4775006594
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3442) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3442&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3442&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3442&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3442&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3442&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3442)
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(registry): add lock protection for serviceListeners map access [dubbo-go]
Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
AlexStocks commented on PR #3442:
URL: https://github.com/apache/dubbo-go/pull/3442#issuecomment-5080589532
CI 失败，解决下
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(registry): add lock protection for serviceListeners map access [dubbo-go]
Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
AlexStocks merged PR #3442:
URL: https://github.com/apache/dubbo-go/pull/3442
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(registry): add lock protection for serviceListeners map access [dubbo-go]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3442:
URL: https://github.com/apache/dubbo-go/pull/3442#issuecomment-5079008271
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3442) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3442&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3442&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3442&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3442&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3442&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3442)
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(registry): add lock protection for serviceListeners map access [dubbo-go]
Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
AlexStocks commented on code in PR #3442:
URL: https://github.com/apache/dubbo-go/pull/3442#discussion_r3435482937
##########
registry/servicediscovery/service_discovery_registry.go:
##########
@@ -362,6 +364,7 @@ func (s *serviceDiscoveryRegistry) SubscribeURL(url *common.URL, notify registry
protocol = url.Protocol
}
protocolServiceKey := url.ServiceKey() + ":" + protocol
+ s.lock.Lock()
Review Comment:
unlock都跑到line 385 了，这太远了，一旦出现 panic 就死锁了，要善于使用 defer + lambda 表达式 啊，童子。我教你个技巧：
func() {
m.lock()
defer m.unlock()
xxx
}()
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(registry): add lock protection for serviceListeners map access [dubbo-go]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #3442:
URL: https://github.com/apache/dubbo-go/pull/3442#issuecomment-4727539340
## [Codecov](https://app.codecov.io/gh/apache/dubbo-go/pull/3442?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 53.57%. Comparing base ([`60d1c2a`](https://app.codecov.io/gh/apache/dubbo-go/commit/60d1c2a949f0ee0da4be3fe09fb79d295491e040?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`19c08a5`](https://app.codecov.io/gh/apache/dubbo-go/commit/19c08a5b03ad35692aeb108550d1222ea4a49a7e?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 847 commits behind head on develop.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## develop #3442 +/- ##
===========================================
+ Coverage 46.76% 53.57% +6.81%
===========================================
Files 295 493 +198
Lines 17172 38390 +21218
===========================================
+ Hits 8031 20569 +12538
- Misses 8287 16169 +7882
- Partials 854 1652 +798
```
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/dubbo-go/pull/3442?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
</details>
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(registry): add lock protection for serviceListeners map access [dubbo-go]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3442:
URL: https://github.com/apache/dubbo-go/pull/3442#issuecomment-4743340588
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3442) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3442&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3442&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3442&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3442&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3442&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3442)
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(registry): add lock protection for serviceListeners map access [dubbo-go]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3442:
URL: https://github.com/apache/dubbo-go/pull/3442#issuecomment-5081269158
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3442) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3442&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3442&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3442&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3442&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3442&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3442)
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(registry): add lock protection for serviceListeners map access [dubbo-go]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3442:
URL: https://github.com/apache/dubbo-go/pull/3442#issuecomment-5081330196
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3442) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3442&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3442&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3442&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3442&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3442&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3442)
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(registry): add lock protection for serviceListeners map access [dubbo-go]
Posted by "Aias00 (via GitHub)" <gi...@apache.org>.
Aias00 commented on code in PR #3442:
URL: https://github.com/apache/dubbo-go/pull/3442#discussion_r3650391890
##########
registry/servicediscovery/service_discovery_registry.go:
##########
@@ -362,6 +364,7 @@ func (s *serviceDiscoveryRegistry) SubscribeURL(url *common.URL, notify registry
protocol = url.Protocol
}
protocolServiceKey := url.ServiceKey() + ":" + protocol
+ s.lock.Lock()
Review Comment:
已改为 `getServiceListener()` helper（内部 `RLock` + `defer RUnlock`），锁只覆盖 `serviceListeners` 的 map 读取，不再跨多行手动 unlock。见 commit 7caf7218。
##########
registry/servicediscovery/service_discovery_registry.go:
##########
@@ -532,23 +537,28 @@ func (s *serviceDiscoveryRegistry) SubscribeURL(url *common.URL, notify registry
protocol = url.Protocol
}
protocolServiceKey := url.ServiceKey() + ":" + protocol
- listener := s.serviceListeners[serviceNamesKey]
- if listener == nil {
- listener = NewServiceInstancesChangedListener(url.GetParam(constant.ApplicationKey, ""), s.url.GetParam(constant.RegistryIdKey, constant.DefaultKey), services)
- for _, serviceNameTmp := range services.Values() {
- serviceName := serviceNameTmp.(string)
- instances := s.serviceDiscovery.GetInstances(serviceName)
- logger.Infof("[Registry][ServiceDiscovery] synchronized instance notification on application %s subscription, instance list size %s", serviceName, len(instances))
- err = listener.OnEvent(®istry.ServiceInstancesChangedEvent{
- ServiceName: serviceName,
- Instances: instances,
- })
- if err != nil {
- logger.Warnf("[Registry][ServiceDiscovery] ServiceInstancesChangedListenerImpl handle error, err=%v", err)
+ var listener registry.ServiceInstancesChangedListener
+ func() {
+ s.lock.Lock()
+ defer s.lock.Unlock()
+ listener = s.serviceListeners[serviceNamesKey]
+ if listener == nil {
+ listener = NewServiceInstancesChangedListener(url.GetParam(constant.ApplicationKey, ""), s.url.GetParam(constant.RegistryIdKey, constant.DefaultKey), services)
+ for _, serviceNameTmp := range services.Values() {
+ serviceName := serviceNameTmp.(string)
+ instances := s.serviceDiscovery.GetInstances(serviceName)
Review Comment:
[P1] 已修。`GetInstances` 与 `listener.OnEvent` 已移出写锁——现在先在锁外构造并加载 listener，再在短写锁内 double-check 后安装，锁只保护 `serviceListeners` 的 check/install。新增 `TestServiceDiscoveryRegistrySubscribeURLDoesNotHoldLockOnExternalCall`：当某个 key 的 `GetInstances` 阻塞时，验证其他 key 的订阅/注销不被全局锁卡住（在旧的"持锁执行 GetInstances"实现下该测试会失败）。见 commit 7caf7218。
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(registry): add lock protection for serviceListeners map access [dubbo-go]
Posted by "Aias00 (via GitHub)" <gi...@apache.org>.
Aias00 commented on PR #3442:
URL: https://github.com/apache/dubbo-go/pull/3442#issuecomment-5081324786
> CI 失败，解决下
CI 失败原因不是 registry 改动本身。失败 job 是 Integration Test，日志里真正退出点是：
sum.golang.org/lookup/github.com/!aias00/dubbo-go/v3@...: 500 Internal Server Error
最近三次 CI 都是同样问题，发生在 go mod tidy 阶段，还没跑到集成测试逻辑
我已经对integrate_test.sh进行了处理
对 fork PR 的 github.com/<fork>/dubbo-go/v3@<sha> replace 设置 GONOSUMDB，避免依赖 sumdb 校验 fork 伪版本
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(registry): add lock protection for serviceListeners map access [dubbo-go]
Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
AlexStocks commented on code in PR #3442:
URL: https://github.com/apache/dubbo-go/pull/3442#discussion_r3610089431
##########
registry/servicediscovery/service_discovery_registry.go:
##########
@@ -532,23 +537,28 @@ func (s *serviceDiscoveryRegistry) SubscribeURL(url *common.URL, notify registry
protocol = url.Protocol
}
protocolServiceKey := url.ServiceKey() + ":" + protocol
- listener := s.serviceListeners[serviceNamesKey]
- if listener == nil {
- listener = NewServiceInstancesChangedListener(url.GetParam(constant.ApplicationKey, ""), s.url.GetParam(constant.RegistryIdKey, constant.DefaultKey), services)
- for _, serviceNameTmp := range services.Values() {
- serviceName := serviceNameTmp.(string)
- instances := s.serviceDiscovery.GetInstances(serviceName)
- logger.Infof("[Registry][ServiceDiscovery] synchronized instance notification on application %s subscription, instance list size %s", serviceName, len(instances))
- err = listener.OnEvent(®istry.ServiceInstancesChangedEvent{
- ServiceName: serviceName,
- Instances: instances,
- })
- if err != nil {
- logger.Warnf("[Registry][ServiceDiscovery] ServiceInstancesChangedListenerImpl handle error, err=%v", err)
+ var listener registry.ServiceInstancesChangedListener
+ func() {
+ s.lock.Lock()
+ defer s.lock.Unlock()
+ listener = s.serviceListeners[serviceNamesKey]
+ if listener == nil {
+ listener = NewServiceInstancesChangedListener(url.GetParam(constant.ApplicationKey, ""), s.url.GetParam(constant.RegistryIdKey, constant.DefaultKey), services)
+ for _, serviceNameTmp := range services.Values() {
+ serviceName := serviceNameTmp.(string)
+ instances := s.serviceDiscovery.GetInstances(serviceName)
Review Comment:
[P1] 不要持有全局 registry 锁执行外部查询和元数据回调
当前写锁从 map 查重一直覆盖 `GetInstances` 和 `listener.OnEvent`。后者会继续执行 `GetMetadataInfo`，其路径可能访问 metadata report/RPC，并重建、通知服务 URL；任一外部调用阻塞时，所有使用同一个 `s.lock` 的注册、注销、订阅和映射更新都会被一起阻塞。这个锁只应保护 `serviceListeners` 的 check/install，不应包住无界外部工作。建议使用按 `serviceNamesKey` 的 singleflight/初始化状态，或先在锁外构造并加载 listener，再在短写锁内 double-check 后安装；同时增加一个让 `GetInstances` 或元数据获取阻塞的并发测试，验证其他 key 的订阅/注销不会被全局锁卡住。
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(registry): add lock protection for serviceListeners map access [dubbo-go]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3442:
URL: https://github.com/apache/dubbo-go/pull/3442#issuecomment-4727511561
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3442) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3442&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3442&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3442&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3442&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3442&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3442)
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(registry): add lock protection for serviceListeners map access [dubbo-go]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3442:
URL: https://github.com/apache/dubbo-go/pull/3442#issuecomment-4743459408
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3442) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3442&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3442&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3442&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3442&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3442&metric=new_duplicated_lines_density&view=list)
<!-- sqra-placement-anchor -->
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3442)
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org