[Bug]Service reference fails when using providedBy after listener destruction

[iwangjie]

Apache Dubbo Component

Java SDK (apache/dubbo)

Dubbo Version

3.1.11

Steps to reproduce this issue

1. Define a service reference with a specific provider:

@DubboReference(providedBy = "app1")
private APIService apiService;

2. Zookeeper has data: get /dubbo/mapping/com.example.APIService value:app1,app2

3. The app1 listener gets destroyed.

4. Try to use another service from app1:

@DubboReference(providedBy = "app1")
private API2Service api2Service;

Result: No provider found.

What you expected to happen

The service reference should succeed.

Anything else

1. Why is the mapping being looked up when the provider is already specified?

2. In the code that handles service instance changes:

if (!oldListener.hasListeners()) {
    oldListener.destroy(); // 🪵
    removeAppSubscriptionLock(appKey);
}

Is it necessary to remove the oldListener from the serviceListeners Map after it has called destroy?

3. Would it be correct to add a line to remove old listeners from serviceListeners, like this:

if (!oldListener.hasListeners()) {
    oldListener.destroy();
    // Remove old listeners from serviceListeners
    serviceListeners.remove(appKey);
    removeAppSubscriptionLock(appKey);
}

Are you willing to submit a pull request to fix on your own?

No

Code of Conduct

• [x] I agree to follow this project's Code of Conduct

---

This consolidated version includes all the key points from your original issue, including the code snippets and your questions about the listener removal process.

[iwangjie]

 for (String newAppName : newApps) {
                    if (!tempOldApps.contains(newAppName)) {
                        serviceNameMapping.removeCachedMapping(ServiceNameMapping.buildMappingKey(url));
                        serviceNameMapping.putCachedMapping(ServiceNameMapping.buildMappingKey(url), newApps);
                        // old instance listener related to old app list that needs to be destroyed after subscribe refresh.
                        ServiceInstancesChangedListener oldListener = listener.getServiceListener();
                        if (oldListener != null) {
                            String appKey = toStringKeys(toTreeSet(tempOldApps));
                            Lock appSubscriptionLock = getAppSubscription(appKey);
                            try {
                                appSubscriptionLock.lock();
                                oldListener.removeListener(url.getServiceKey(), listener);
                                if (!oldListener.hasListeners()) {
                                    oldListener.destroy(); // 🪵
                                    removeAppSubscriptionLock(appKey);
                                }
                            } finally {
                                appSubscriptionLock.unlock();
                            }
                        }

                        subscribeURLs(url, listener, newApps);
                        oldApps = newApps;
                        return;
                    }
                }

Is it necessary to remove the oldListener from the serviceListeners Map after it has called destroy?

[iwangjie]

After experimenting, if I add a new lineRemove old listeners from serviceListeners. Is it right to do so?

 if (!oldListener.hasListeners()) {
      oldListener.destroy();
      // Remove old listeners from serviceListeners
      serviceListeners.remove(appKey);
      removeAppSubscriptionLock(appKey);
  }

[wcy666103]

PTAL @AlbumenJ

[AlbumenJ]

@chickenlj PTAL

[chickenlj]

• Why is the mapping being looked up when the provider is already specified?

When the interface-app mapping is set by providedBy for one service/interface, the changing notification from MappingListener should not affect this service/interface. This is the main problem you have encountered, right?

[iwangjie]

• Why is the mapping being looked up when the provider is already specified?

When the interface-app mapping is set by providedBy for one service/interface, the changing notification from MappingListener should not affect this service/interface. This is the main problem you have encountered, right?

When providedBy is specified, shouldn't it override the registry mapping data? We’ve added providedBy to each reference for future cloud-native service grids, but I'm unsure how to prevent Dubbo from using the mapping data as the default.

In other words, I want the providedBy attribute of the service to be set explicitly by me. It should use the mapping data only if providedBy isn't specified, or ideally, it should report an error if providedBy is not provided.

[chickenlj]

Thanks for the detailed information, I see the problem here. How to guarantee the priority of providedBy need to be further investigated.

Have you tried the latest 3.2.14 or 3.2.15 release? I think even if providedBy gets overridden unexpectedly, it should not report no provider exception, it should still has available providers, only the providers change from app1 to app1,app2

[iwangjie]

Thanks for the detailed information, I see the problem here. How to guarantee the priority of providedBy need to be further investigated.

Have you tried the latest 3.2.14 or 3.2.15 release? I think even if providedBy gets overridden unexpectedly, it should not report no provider exception, it should still has available providers, only the providers change from app1 to app1,app2

I can also provide what I believe is a critical piece of information. It seems there might be a conflict between the timing of reference loading and the Mapping listener. Let me give you an example.

Firstly, we have defined two references that point to different interfaces of the same app. Due to the order of loading and internal mechanisms, when UserService is injected, the Mapping listener retrieves mappings from zk as app1, app2. At that moment, it happens to send a MappingChangeEvent, which internally triggers the destruction of the listener for app1 (without removing it). When PicService loads, it specifies 'providedBy', so it retrieves the destroyed listener from the Mapping cache. Since the listener has already been destroyed(Listener of %s has been destroyed by another thread.), PicService simply ignores the registration and does nothing.

@DubboReference(providedBy = "app1") private UserService userService;

@DubboReference(providedBy = "app1") private PicService picService;

    @Override
    public synchronized void initInterfaceAppMapping(URL subscribedURL) {
        String key = ServiceNameMapping.buildMappingKey(subscribedURL);
        if (hasInitiated(key)) {
            return;
        }
        mappingInitStatus.put(key, Boolean.TRUE);

        Set<String> subscribedServices = new TreeSet<>();
        String serviceNames = subscribedURL.getParameter(PROVIDED_BY); // ⚠️ Pay attention here

        if (StringUtils.isNotEmpty(serviceNames)) {
            logger.info(key + " mapping to " + serviceNames + " instructed by provided-by set by user.");
            subscribedServices.addAll(parseServices(serviceNames));
        }

        if (isEmpty(subscribedServices)) {
            Set<String> cachedServices = this.getCachedMapping(key);
            if (!isEmpty(cachedServices)) {
                logger.info(key + " mapping to " + serviceNames + " instructed by local cache.");
                subscribedServices.addAll(cachedServices);
            }
        } else {
            this.putCachedMappingIfAbsent(key, subscribedServices);
        }
    }

    protected void subscribeURLs(URL url, NotifyListener listener, Set<String> serviceNames) {
        serviceNames = toTreeSet(serviceNames);
        String serviceNamesKey = toStringKeys(serviceNames);
        String serviceKey = url.getServiceKey();
        logger.info(String.format("Trying to subscribe from apps %s for service key %s, ", serviceNamesKey, serviceKey));

        // register ServiceInstancesChangedListener
        Lock appSubscriptionLock = getAppSubscription(serviceNamesKey);
        try {
            appSubscriptionLock.lock();
            ServiceInstancesChangedListener serviceInstancesChangedListener = serviceListeners.get(serviceNamesKey);
            if (serviceInstancesChangedListener == null) {
                serviceInstancesChangedListener = serviceDiscovery.createListener(serviceNames);
                serviceInstancesChangedListener.setUrl(url);
                for (String serviceName : serviceNames) {
                    List<ServiceInstance> serviceInstances = serviceDiscovery.getInstances(serviceName);
                    if (CollectionUtils.isNotEmpty(serviceInstances)) {
                        serviceInstancesChangedListener.onEvent(new ServiceInstancesChangedEvent(serviceName, serviceInstances));
                    }
                }
                serviceListeners.put(serviceNamesKey, serviceInstancesChangedListener);
            }

            if (!serviceInstancesChangedListener.isDestroyed()) {
                serviceInstancesChangedListener.setUrl(url);
                listener.addServiceListener(serviceInstancesChangedListener);
                serviceInstancesChangedListener.addListenerAndNotify(url, listener);
                serviceDiscovery.addServiceInstancesChangedListener(serviceInstancesChangedListener);
            } else {
                logger.info(String.format("Listener of %s has been destroyed by another thread.", serviceNamesKey)); // The code goes here
                serviceListeners.remove(serviceNamesKey);
            }
        } finally {
            appSubscriptionLock.unlock();
        }
    }

[chickenlj]

Thanks for the input, I will look into it later this week.