Copilot commented on code in PR #3367:
URL: https://github.com/apache/dubbo-go/pull/3367#discussion_r3367022869
##########
metadata/report_instance.go:
##########
@@ -47,11 +49,19 @@ func addMetadataReport(registryId string, url *common.URL)
error {
logger.Warnf("no metadata report factory of protocol %s found,
please check if the metadata report factory is imported", url.Protocol)
return nil
}
+ instancesMu.Lock()
instances[registryId] = &DelegateMetadataReport{instance:
fac.CreateMetadataReport(url)}
+ instancesMu.Unlock()
Review Comment:
addMetadataReport holds instancesMu while calling
fac.CreateMetadataReport(url). Factory creation may be slow or may block (e.g.,
dialing/config loading), so doing it under the global lock can unnecessarily
stall readers and other writers. Create the report instance first, then take
the lock only to publish it into the map.
##########
metadata/info/metadata_info.go:
##########
@@ -155,13 +172,28 @@ func (info *MetadataInfo) GetExportedServiceURLs()
[]*common.URL {
}
func (info *MetadataInfo) GetSubscribedURLs() []*common.URL {
+ info.mu.RLock()
+ defer info.mu.RUnlock()
+
res := make([]*common.URL, 0)
for _, urls := range info.subscribedServiceURLs {
res = append(res, urls...)
}
return res
}
+// GetServices returns a copy of the Services map for safe iteration by
external callers.
+func (info *MetadataInfo) GetServices() map[string]*ServiceInfo {
+ info.mu.RLock()
+ defer info.mu.RUnlock()
+
+ cp := make(map[string]*ServiceInfo, len(info.Services))
+ for k, v := range info.Services {
+ cp[k] = v
+ }
+ return cp
+}
Review Comment:
GetServices currently returns a shallow copy of the Services map but reuses
the original *ServiceInfo pointers. Because ServiceInfo has lazily-populated
fields (e.g., GetMatchKey/GetServiceKey write back to MatchKey/ServiceKey when
empty), concurrent readers iterating over the snapshot can still race on those
shared ServiceInfo objects. Consider returning deep-copied ServiceInfo values
(and Params map) and eagerly populating derived keys in the snapshot to avoid
any write-on-read behavior.
--
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]
