[BUG] URL struct memory leaks: attributes never deleted, listeners never cleaned, blacklist key mismatch

[AlexStocks]

Verification Checklist

• I have searched the existing Issues and believe this is not a duplicate

Go Version

1.24

Dubbo-go Version

v3.3.1

OS

Linux / All platforms

Bug Description

After a thorough code audit of the common.URL struct and its lifecycle management across the codebase, I identified 10 memory leak points (1 P0, 5 P1, 4 P2). These leaks cause unbounded memory growth in long-running services, especially in environments with frequent service registration/deregistration or configuration changes.

---

P0: registryProtocol singleton leaks overrideListeners and serviceConfigurationListeners

File: registry/protocol/protocol.go:59-70, 457-496

type registryProtocol struct {
    overrideListeners             *sync.Map  // leaked
    serviceConfigurationListeners *sync.Map  // leaked
}

Export() (L218, L221) stores listeners into these maps, but Destroy() (L457-496) only cleans bounds and registries — never cleans overrideListeners or serviceConfigurationListeners. Each overrideSubscribeListener holds a reference to the full originInvoker (including URL and connection resources). Since registryProtocol is a package-level singleton (L50 regProtocol), these references are never GC'd.

Impact: Every exported service leaks its listener + invoker + URL + connections. In microservice environments with frequent service lifecycle changes, memory grows proportionally to total services ever exported.

Fix: Add Range + Delete cleanup for both maps in Destroy(). Also Delete old listeners in reExport.

---

P1-1: URL attributes are write-only — no DeleteAttribute method exists

File: common/url.go:583-590

func (c *URL) SetAttribute(key string, value any) {
    c.attributes[key] = value  // write-only, no delete API
}

The entire codebase has zero calls to delete attributes. Values stored include large objects: *global.ApplicationConfig, map[string]*global.RegistryConfig, ServiceInfo, RPCService implementations. The filter/adaptivesvc/filter.go:96 writes an updater on every RPC call.

Fix: Add DeleteAttribute(key string). Clean attributes on invoker/exporter Destroy.

---

P1-2: RegistryDirectory.Destroy() doesn't clean cacheInvokersMap

File: registry/directory/directory.go:549-573

func (dir *RegistryDirectory) Destroy() {
    dir.DoDestroy(func() {
        invokers := dir.cacheInvokers
        dir.cacheInvokers = []protocolbase.Invoker{}
        for _, ivk := range invokers {
            ivk.Destroy()
        }
        // cacheInvokersMap (sync.Map) is NEVER cleaned!
    })
}

The sync.Map still holds references to all destroyed invokers + their URLs. If RegistryDirectory is held by the singleton registryProtocol, these references leak permanently.

Fix: Add dir.cacheInvokersMap.Range(func(k, _ any) bool { dir.cacheInvokersMap.Delete(k); return true }).

---

P1-3: invokerBlackList key mismatch prevents cleanup

File: protocol/base/rpc_status.go:40, 213

// Store uses URL.Key()
func SetInvokerUnhealthyStatus(invoker Invoker) {
    invokerBlackList.Store(invoker.GetURL().Key(), invoker)
}

// Delete uses GetCacheInvokerMapKey() — different format!
func RemoveUrlKeyUnhealthyStatus(key string) {
    invokerBlackList.Delete(key)
}

SetInvokerUnhealthyStatus uses URL.Key() as the map key, but RemoveUrlKeyUnhealthyStatus receives keys from GetCacheInvokerMapKey() which has a different format. The Delete never matches, so blacklisted invokers (holding full invoker + URL + connections) are never removed.

Fix: Unify key generation. Or store only the key string, not the full invoker object.

---

P1-4: GetParams() returns internal map reference without lock or copy

File: common/url.go:657-659

func (c *URL) GetParams() url.Values {
    return c.params  // no lock, no copy
}

Callers hold the internal params reference indefinitely, preventing URL GC. Callers also modify it directly (e.g., base_configuration_listener.go:111 does delete(override, constant.AnyhostKey) on the returned map), which is a concurrent mutation without any lock.

Fix: Return a copy, or deprecate in favor of CopyParams().

---

P1-5: configurators slice only grows, never shrinks

File: registry/directory/directory.go:379

dir.configurators = append(dir.configurators, extension.GetDefaultConfigurator(ret))

Every override/configurator event appends a new Configurator (holding a *common.URL). Never replaced or trimmed. In long-running systems with frequent config pushes, this slice grows unbounded.

Fix: Replace entire slice on new config (like BaseConfigurationListener.Process does at L88), not append.

---

P2-1: MergeURL reads anotherUrl.attributes without lock

File: common/url.go:879-891

for attrK, attrV := range anotherUrl.attributes {  // no attributesLock!
    if _, ok := mergedURL.GetAttribute(attrK); !ok {
        mergedURL.attributes[attrK] = attrV
    }
}

This is both a data race and a reference leak (shallow copy extends object lifetimes).

---

P2-2: GetCacheInvokerMapKey creates a temporary URL on every call

File: common/url.go:418-428

func (c *URL) GetCacheInvokerMapKey() string {
    urlNew, _ := NewURL(c.PrimitiveURL)  // allocates a full URL object
    // just to extract timestamp
}

Called on every ServiceEvent.Key() in hot paths. Not a leak, but unnecessary GC pressure.

---

P2-3: AddParam vs SetParam semantic confusion

File: common/url.go:537-543, registry/protocol/protocol.go:400

// isMatched() calls this repeatedly, accumulating values under same key
providerUrl.AddParam(constant.CategoryKey, constant.ConfiguratorsCategory)

url.Values.Add appends (not overwrites). Each isMatched call adds another CategoryKey value, causing params[CategoryKey] to grow linearly.

Fix: Use SetParam instead of AddParam in isMatched.

---

P2-4: registryProtocol.Destroy() missing else branch in async cleanup

File: registry/protocol/protocol.go:457-496

go func() {
    if configShutdown := config.GetShutDown(); configShutdown != nil {
        // cleanup path 1
    }
    if shutdownConfRaw, ok := ...; ok {
        // cleanup path 2
    }
    // NO else: if both conditions fail, goroutine exits without UnExport or Delete
}()

Fix: Add else branch that unconditionally UnExport + Delete.

---

Summary

#	Issue	Severity	Type
1	overrideListeners/serviceConfigurationListeners never cleaned	P0	Leak
2	attributes write-only, no delete API	P1	Design
3	cacheInvokersMap not cleaned on Destroy	P1	Leak
4	invokerBlackList key mismatch	P1	Leak
5	GetParams() returns internal reference	P1	Leak + Race
6	configurators only appended, never replaced	P1	Leak
7	MergeURL attributes access without lock	P2	Race + Leak
8	GetCacheInvokerMapKey allocates temp URL	P2	GC pressure
9	AddParam/SetParam confusion	P2	Accumulation
10	Destroy async cleanup missing else	P2	Conditional leak

Suggested Fix Priority

1. Fix P0 first (listener maps cleanup in registryProtocol.Destroy)
2. Add DeleteAttribute API and clean attributes on Destroy
3. Unify blacklist key generation
4. Fix GetParams() to return copy
5. Replace append with full replacement for configurators

[AlexStocks]

Concrete Fix Proposals (with code)

Below are precise, copy-paste-ready fixes for each leak point. Ordered by priority.

---

Fix 1 (P0): registryProtocol.Destroy() — clean overrideListeners and serviceConfigurationListeners

Current code (registry/protocol/protocol.go:457-496):

func (proto *registryProtocol) Destroy() {
    proto.bounds.Range(func(key, value any) bool {
        exporter := value.(*exporterChangeableWrapper)
        // ... UnRegister + async UnExport ...
        return true
    })
    proto.bounds = &sync.Map{}
    proto.registries = &sync.Map{}
    // overrideListeners and serviceConfigurationListeners are NEVER cleaned
}

Fix — add cleanup at the end of Destroy():

func (proto *registryProtocol) Destroy() {
    // ... existing bounds cleanup ...

    // Clean override listeners to release originInvoker references
    proto.overrideListeners.Range(func(key, value any) bool {
        proto.overrideListeners.Delete(key)
        return true
    })

    // Clean service configuration listeners to release providerUrl references
    proto.serviceConfigurationListeners.Range(func(key, value any) bool {
        proto.serviceConfigurationListeners.Delete(key)
        return true
    })

    proto.bounds = &sync.Map{}
    proto.registries = &sync.Map{}
}

Also fix reExport() — delete old listener before storing new one:

// registry/protocol/protocol.go, in reExport() around L380-400
// Before storing new listener, delete old one:
func (proto *registryProtocol) reExport(invoker base.Invoker, newUrl *common.URL) {
    key := getCacheKey(invoker)
    // Delete old override listener if exists
    if oldListener, loaded := proto.overrideListeners.LoadAndDelete(getProviderUrl(invoker)); loaded {
        _ = oldListener // allow GC
    }
    // ... rest of reExport ...
}

---

Fix 2 (P1): Add DeleteAttribute and ClearAttributes to URL

Current code (common/url.go:583-597):

func (c *URL) SetAttribute(key string, value any) {
    c.attributesLock.Lock()
    defer c.attributesLock.Unlock()
    if c.attributes == nil {
        c.attributes = make(map[string]any)
    }
    c.attributes[key] = value
}

func (c *URL) GetAttribute(key string) (any, bool) {
    c.attributesLock.RLock()
    defer c.attributesLock.RUnlock()
    v, ok := c.attributes[key]
    return v, ok
}

Fix — add two new methods right after GetAttribute:

// DeleteAttribute removes the attribute with the given key.
// Use this in Destroy() paths to release references held by URL.
func (c *URL) DeleteAttribute(key string) {
    c.attributesLock.Lock()
    defer c.attributesLock.Unlock()
    delete(c.attributes, key)
}

// ClearAttributes removes all attributes.
// Use this when the URL is being destroyed and all references should be released.
func (c *URL) ClearAttributes() {
    c.attributesLock.Lock()
    defer c.attributesLock.Unlock()
    c.attributes = nil
}

Where to call them — in every Destroy() that holds a URL:

// protocol/base/protocol.go, in BaseProtocol.Destroy() or each protocol's Destroy:
func (ivk *BaseInvoker) Destroy() {
    // ... existing cleanup ...
    if ivk.url != nil {
        ivk.url.ClearAttributes()
    }
}

// registry/directory/directory.go, in RegistryDirectory.Destroy():
func (dir *RegistryDirectory) Destroy() {
    dir.DoDestroy(func() {
        // ... existing cleanup ...
        invokers := dir.cacheInvokers
        dir.cacheInvokers = []protocolbase.Invoker{}
        for _, ivk := range invokers {
            ivk.GetURL().ClearAttributes() // release attribute references before Destroy
            ivk.Destroy()
        }
    })
}

---

Fix 3 (P1): RegistryDirectory.Destroy() — clean cacheInvokersMap

Current code (registry/directory/directory.go:549-573):

func (dir *RegistryDirectory) Destroy() {
    dir.DoDestroy(func() {
        // ...
        invokers := dir.cacheInvokers
        dir.cacheInvokers = []protocolbase.Invoker{}
        for _, ivk := range invokers {
            ivk.Destroy()
        }
        // cacheInvokersMap (sync.Map) is NEVER cleaned!
    })
}

Fix — add cleanup right after the invoker loop:

func (dir *RegistryDirectory) Destroy() {
    dir.DoDestroy(func() {
        // ...
        invokers := dir.cacheInvokers
        dir.cacheInvokers = []protocolbase.Invoker{}
        for _, ivk := range invokers {
            ivk.Destroy()
        }

        // Clean cacheInvokersMap to release all invoker references
        dir.cacheInvokersMap.Range(func(key, _ any) bool {
            dir.cacheInvokersMap.Delete(key)
            return true
        })

        // Also nil out configurators to release Configurator -> URL references
        dir.configurators = nil
    })
}

---

Fix 4 (P1): invokerBlackList key mismatch — unify key generation

Root cause: SetInvokerUnhealthyStatus uses invoker.GetURL().Key(), but RemoveUrlKeyUnhealthyStatus receives keys from GetCacheInvokerMapKey() which has a different format.

Current code (protocol/base/rpc_status.go:212-243):

func SetInvokerUnhealthyStatus(invoker Invoker) {
    invokerBlackList.Store(invoker.GetURL().Key(), invoker)  // Key() format
}

func RemoveUrlKeyUnhealthyStatus(key string) {
    invokerBlackList.Delete(key)  // key comes from GetCacheInvokerMapKey() format
}

Caller (registry/directory/directory.go:457):

func (dir *RegistryDirectory) uncacheInvokerWithKey(key string) {
    // key = ServiceEvent.Key() = URL.GetCacheInvokerMapKey()
    protocolbase.RemoveUrlKeyUnhealthyStatus(key)
}

Two-part fix:

Option A — Store only key string, not invoker object; unify key format:

// rpc_status.go

// Change blacklist to store URL.Key() -> struct{} (not full invoker)
// and add a separate method that accepts URL directly

func SetInvokerUnhealthyStatus(invoker Invoker) {
    key := invoker.GetURL().Key()
    invokerBlackList.Store(key, struct{}{})  // don't store full invoker
    blackListCacheDirty.Store(true)
    logger.Infof("%.10s add to black list", key)
}

func RemoveInvokerUnhealthyStatus(invoker Invoker) {
    key := invoker.GetURL().Key()
    invokerBlackList.Delete(key)
    blackListCacheDirty.Store(true)
    logger.Infof("%.10s removed from black list", key)
}

// Keep RemoveUrlKeyUnhealthyStatus but convert key format
func RemoveUrlKeyUnhealthyStatus(cacheKey string) {
    // Also try to find and delete by iterating (since key formats differ)
    invokerBlackList.Range(func(k, _ any) bool {
        if strings.Contains(k.(string), extractServiceKey(cacheKey)) {
            invokerBlackList.Delete(k)
            blackListCacheDirty.Store(true)
            return false // found, stop
        }
        return true
    })
}

Option B (simpler, recommended) — make uncacheInvokerWithKey pass the correct key:

// registry/directory/directory.go
func (dir *RegistryDirectory) uncacheInvokerWithKey(key string) {
    logger.Debugf("%.10s will be uncached", key)
    if cacheInvoker, ok := dir.cacheInvokersMap.Load(key); ok {
        dir.cacheInvokersMap.Delete(key)
        invoker := cacheInvoker.(protocolbase.Invoker)
        // Use the invoker's own URL.Key() to remove from blacklist
        protocolbase.RemoveUrlKeyUnhealthyStatus(invoker.GetURL().Key())
        invoker.Destroy()
    }
}

---

Fix 5 (P1): GetParams() — return copy instead of internal reference

Current code (common/url.go:657-659):

func (c *URL) GetParams() url.Values {
    return c.params
}

Fix:

// GetParams returns a deep copy of the URL parameters.
// Callers can safely modify the returned map without affecting the URL.
func (c *URL) GetParams() url.Values {
    c.paramsLock.RLock()
    defer c.paramsLock.RUnlock()
    if c.params == nil {
        return url.Values{}
    }
    copy := make(url.Values, len(c.params))
    for k, vs := range c.params {
        vsCopy := make([]string, len(vs))
        for i, v := range vs {
            vsCopy[i] = v
        }
        copy[k] = vsCopy
    }
    return copy
}

Impact: Callers that relied on mutating the returned map to affect the URL will break. Audit these callers:

// registry/base_configuration_listener.go:111 — currently mutates returned map
override := url.GetParams()
delete(override, constant.AnyhostKey)  // after fix: only deletes from copy, URL unchanged
// FIX: if intent is to modify URL, use url.DelParam(constant.AnyhostKey) instead

// config_center/configurator/override.go:88 — uses SetParams(configUrl.GetParams())
// After fix: this still works correctly since SetParams deep-copies its input

---

Fix 6 (P1): configurators — replace instead of append

Current code (registry/directory/directory.go:375-386):

func (dir *RegistryDirectory) convertUrl(res *registry.ServiceEvent) *common.URL {
    ret := res.Service
    if ret.Protocol == constant.OverrideProtocol ||
        ret.GetParam(constant.CategoryKey, constant.DefaultCategory) == constant.ConfiguratorsCategory {
        dir.configurators = append(dir.configurators, extension.GetDefaultConfigurator(ret))
        ret = nil
    }
    // ...
}

Fix — follow the same pattern as BaseConfigurationListener.Process() (L88), which replaces the entire slice:

func (dir *RegistryDirectory) convertUrl(res *registry.ServiceEvent) *common.URL {
    ret := res.Service
    if ret.Protocol == constant.OverrideProtocol ||
        ret.GetParam(constant.CategoryKey, constant.DefaultCategory) == constant.ConfiguratorsCategory {
        // Replace configurator for this service key, not append
        newConfigurator := extension.GetDefaultConfigurator(ret)
        dir.replaceConfigurator(newConfigurator)
        ret = nil
    }
    // ...
}

// replaceConfigurator replaces existing configurator for the same URL key,
// or appends if no matching configurator exists.
func (dir *RegistryDirectory) replaceConfigurator(newConf config_center.Configurator) {
    newKey := newConf.GetUrl().Key()
    for i, existing := range dir.configurators {
        if existing.GetUrl().Key() == newKey {
            dir.configurators[i] = newConf
            return
        }
    }
    dir.configurators = append(dir.configurators, newConf)
}

---

Fix 7 (P2): MergeURL — add lock when reading anotherUrl.attributes

Current code (common/url.go:879-891):

// merge attributes
if mergedURL.attributes == nil {
    mergedURL.attributes = make(map[string]any, len(anotherUrl.attributes))
}
for attrK, attrV := range anotherUrl.attributes {  // NO LOCK on anotherUrl!
    if _, ok := mergedURL.GetAttribute(attrK); !ok {
        mergedURL.attributes[attrK] = attrV
    }
}

Fix:

// merge attributes
anotherUrl.attributesLock.RLock()
anotherAttrs := make(map[string]any, len(anotherUrl.attributes))
for k, v := range anotherUrl.attributes {
    anotherAttrs[k] = v
}
anotherUrl.attributesLock.RUnlock()

if len(anotherAttrs) > 0 {
    if mergedURL.attributes == nil {
        mergedURL.attributes = make(map[string]any, len(anotherAttrs))
    }
    for attrK, attrV := range anotherAttrs {
        if _, ok := mergedURL.GetAttribute(attrK); !ok {
            mergedURL.attributes[attrK] = attrV
        }
    }
}

---

Fix 8 (P2): GetCacheInvokerMapKey — avoid allocating temporary URL

Current code (common/url.go:418-428):

func (c *URL) GetCacheInvokerMapKey() string {
    urlNew, _ := NewURL(c.PrimitiveURL)  // allocates entire URL just to read one param
    buildString := urlNew.Protocol + "://" + urlNew.GetParam(constant.HostnameKey, urlNew.Location) +
        "/" + c.GetParam(constant.InterfaceKey, c.Path) +
        "?" + constant.TimeStamp + "=" + c.GetParam(constant.RemoteTimestampKey, "") +
        "&" + constant.MeshClusterIDKey + "=" + c.GetParam(constant.MeshClusterIDKey, "")
    return buildString
}

Fix — read HostnameKey directly from params, no need for NewURL:

func (c *URL) GetCacheInvokerMapKey() string {
    hostname := c.GetParam(constant.HostnameKey, c.Location)
    return c.Protocol + "://" + hostname +
        "/" + c.GetParam(constant.InterfaceKey, c.Path) +
        "?" + constant.TimeStamp + "=" + c.GetParam(constant.RemoteTimestampKey, "") +
        "&" + constant.MeshClusterIDKey + "=" + c.GetParam(constant.MeshClusterIDKey, "")
}

If PrimitiveURL contains a different hostname than the parsed Location, it means the URL parsing was inconsistent. In that case, c.GetParam(constant.HostnameKey, c.Location) is the safer fallback anyway.

---

Fix 9 (P2): isMatched — use SetParam instead of AddParam

Current code (registry/protocol/protocol.go:400):

func isMatched(party *common.URL, url *common.URL) bool {
    // ...
    providerUrl.AddParam(constant.CategoryKey, constant.ConfiguratorsCategory)  // appends, accumulates
    // ...
}

Fix:

providerUrl.SetParam(constant.CategoryKey, constant.ConfiguratorsCategory)  // overwrites, no accumulation

---

Fix 10 (P2): registryProtocol.Destroy() — add else branch for async cleanup

Current code (registry/protocol/protocol.go:468-491):

go func() {
    if configShutdown := config.GetShutDown(); configShutdown != nil {
        <-time.After(configShutdown.GetStepTimeout() + configShutdown.GetConsumerUpdateWaitTime())
        exporter.UnExport()
        proto.bounds.Delete(key)
        return
    }
    if shutdownConfRaw, ok := exporter.registerUrl.GetAttribute(constant.ShutdownConfigPrefix); ok {
        if shutdownConf, ok := shutdownConfRaw.(*global.ShutdownConfig); ok {
            // ... same pattern ...
            return
        }
    }
    // BUG: if both conditions fail, nothing happens — exporter leaks
}()

Fix — add else branch:

go func() {
    if configShutdown := config.GetShutDown(); configShutdown != nil {
        <-time.After(configShutdown.GetStepTimeout() + configShutdown.GetConsumerUpdateWaitTime())
        exporter.UnExport()
        proto.bounds.Delete(key)
        return
    }
    if shutdownConfRaw, ok := exporter.registerUrl.GetAttribute(constant.ShutdownConfigPrefix); ok {
        if shutdownConf, ok := shutdownConfRaw.(*global.ShutdownConfig); ok {
            // ... same pattern ...
            return
        }
    }
    // Fallback: no shutdown config found, clean up immediately
    logger.Warnf("No shutdown config found for exporter %s, cleaning up immediately", key)
    exporter.UnExport()
    proto.bounds.Delete(key)
}()

[nagisa-kunhah]

hello, can assign it to me？

[AlexStocks]

hello, can assign it to me？

It's the deal. thank you for your contribution. do it.