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: [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]
