You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dubbo.apache.org by li...@apache.org on 2022/05/10 01:40:22 UTC

[dubbo] branch 3.0 updated: Followup of #10010, share proxy instance among References that have the same key. (#10017)

This is an automated email from the ASF dual-hosted git repository.

liujun pushed a commit to branch 3.0
in repository https://gitbox.apache.org/repos/asf/dubbo.git


The following commit(s) were added to refs/heads/3.0 by this push:
     new d0ae161635 Followup of #10010, share proxy instance among References that have the same key. (#10017)
d0ae161635 is described below

commit d0ae1616353af79ea2f9d842bcf04bdc68bfb24e
Author: ken.lj <ke...@gmail.com>
AuthorDate: Tue May 10 09:40:16 2022 +0800

    Followup of #10010, share proxy instance among References that have the same key. (#10017)
    
    fixes #10012
---
 .../dubbo/config/AbstractInterfaceConfig.java      | 22 +++++++++++--
 .../dubbo/config/utils/SimpleReferenceCache.java   | 36 +++++++++++++++++++---
 .../dubbo/config/utils/ReferenceCacheTest.java     | 13 +++++---
 3 files changed, 59 insertions(+), 12 deletions(-)

diff --git a/dubbo-common/src/main/java/org/apache/dubbo/config/AbstractInterfaceConfig.java b/dubbo-common/src/main/java/org/apache/dubbo/config/AbstractInterfaceConfig.java
index 72cd1da214..5e9b8b05f8 100644
--- a/dubbo-common/src/main/java/org/apache/dubbo/config/AbstractInterfaceConfig.java
+++ b/dubbo-common/src/main/java/org/apache/dubbo/config/AbstractInterfaceConfig.java
@@ -187,7 +187,15 @@ public abstract class AbstractInterfaceConfig extends AbstractMethodConfig {
 
     protected String tag;
 
-    private  Boolean auth;
+    private Boolean auth;
+
+    /*Indicates to create separate instances or not for services/references that have the same serviceKey.
+     * By default, all services/references that have the same serviceKey will share the same instance and process.
+     *
+     * This key currently can only work when using ReferenceConfig and SimpleReferenceCache together.
+     * Call ReferenceConfig.get() directly will not check this attribute.
+     */
+    private Boolean singleton;
 
     public AbstractInterfaceConfig() {
     }
@@ -855,14 +863,22 @@ public abstract class AbstractInterfaceConfig extends AbstractMethodConfig {
     public SslConfig getSslConfig() {
         return getConfigManager().getSsl().orElse(null);
     }
-    
+
+    public Boolean getSingleton() {
+        return singleton;
+    }
+
+    public void setSingleton(Boolean singleton) {
+        this.singleton = singleton;
+    }
+
     protected void initServiceMetadata(AbstractInterfaceConfig interfaceConfig) {
         serviceMetadata.setVersion(getVersion(interfaceConfig));
         serviceMetadata.setGroup(getGroup(interfaceConfig));
         serviceMetadata.setDefaultGroup(getGroup(interfaceConfig));
         serviceMetadata.setServiceInterfaceName(getInterface());
     }
-    
+
     public String getGroup(AbstractInterfaceConfig interfaceConfig) {
         return StringUtils.isEmpty(getGroup()) ? (interfaceConfig != null ? interfaceConfig.getGroup() : getGroup()) : getGroup();
     }
diff --git a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/utils/SimpleReferenceCache.java b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/utils/SimpleReferenceCache.java
index 893799719c..34aae97d1a 100644
--- a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/utils/SimpleReferenceCache.java
+++ b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/utils/SimpleReferenceCache.java
@@ -18,6 +18,8 @@ package org.apache.dubbo.config.utils;
 
 import org.apache.dubbo.common.BaseServiceMetadata;
 import org.apache.dubbo.common.config.ReferenceCache;
+import org.apache.dubbo.common.logger.Logger;
+import org.apache.dubbo.common.logger.LoggerFactory;
 import org.apache.dubbo.common.utils.CollectionUtils;
 import org.apache.dubbo.common.utils.StringUtils;
 import org.apache.dubbo.config.ReferenceConfigBase;
@@ -40,6 +42,7 @@ import java.util.concurrent.atomic.AtomicInteger;
  * You can implement and use your own {@link ReferenceConfigBase} cache if you need use complicate strategy.
  */
 public class SimpleReferenceCache implements ReferenceCache {
+    private static final Logger logger = LoggerFactory.getLogger(SimpleReferenceCache.class);
     public static final String DEFAULT_NAME = "_DEFAULT_";
     /**
      * Create the key with the <b>Group</b>, <b>Interface</b> and <b>version</b> attribute of {@link ReferenceConfigBase}.
@@ -107,15 +110,26 @@ public class SimpleReferenceCache implements ReferenceCache {
     public <T> T get(ReferenceConfigBase<T> rc) {
         String key = generator.generateKey(rc);
         Class<?> type = rc.getInterfaceClass();
-        Object proxy = references.computeIfAbsent(rc, _rc -> {
+
+        boolean singleton = rc.getSingleton() == null || rc.getSingleton();
+        T proxy = null;
+        // Check existing proxy of the same 'key' and 'type' first.
+        if (singleton) {
+            proxy = get(key, (Class<T>) type);
+        } else {
+            logger.warn("Using non-singleton ReferenceConfig and ReferenceCache at the same time may cause memory leak. " +
+                "Call ReferenceConfig#get() directly for non-singleton ReferenceConfig instead of using ReferenceCache#get(ReferenceConfig)");
+        }
+
+        if (proxy == null) {
             List<ReferenceConfigBase<?>> referencesOfType = referenceTypeMap.computeIfAbsent(type, _t -> Collections.synchronizedList(new ArrayList<>()));
             referencesOfType.add(rc);
             List<ReferenceConfigBase<?>> referenceConfigList = referenceKeyMap.computeIfAbsent(key, _k -> Collections.synchronizedList(new ArrayList<>()));
             referenceConfigList.add(rc);
-            return _rc.get();
-        });
+            proxy = rc.get();
+        }
 
-        return (T) proxy;
+        return proxy;
     }
 
     /**
@@ -138,6 +152,13 @@ public class SimpleReferenceCache implements ReferenceCache {
         return null;
     }
 
+    /**
+     * Check and return existing ReferenceConfig and its corresponding proxy instance.
+     *
+     * @param key ServiceKey
+     * @param <T> service interface type
+     * @return the existing proxy instance of the same service key
+     */
     @Override
     @SuppressWarnings("unchecked")
     public <T> T get(String key) {
@@ -162,6 +183,13 @@ public class SimpleReferenceCache implements ReferenceCache {
         return Collections.unmodifiableList(proxiesOfType);
     }
 
+    /**
+     * Check and return existing ReferenceConfig and its corresponding proxy instance.
+     *
+     * @param type service interface class
+     * @param <T>  service interface type
+     * @return the existing proxy instance of the same interface definition
+     */
     @Override
     @SuppressWarnings("unchecked")
     public <T> T get(Class<T> type) {
diff --git a/dubbo-config/dubbo-config-api/src/test/java/org/apache/dubbo/config/utils/ReferenceCacheTest.java b/dubbo-config/dubbo-config-api/src/test/java/org/apache/dubbo/config/utils/ReferenceCacheTest.java
index 84d2029ef5..e0174dec39 100644
--- a/dubbo-config/dubbo-config-api/src/test/java/org/apache/dubbo/config/utils/ReferenceCacheTest.java
+++ b/dubbo-config/dubbo-config-api/src/test/java/org/apache/dubbo/config/utils/ReferenceCacheTest.java
@@ -26,6 +26,7 @@ import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 public class ReferenceCacheTest {
@@ -42,16 +43,18 @@ public class ReferenceCacheTest {
         ReferenceCache cache = SimpleReferenceCache.getCache();
         MockReferenceConfig config = buildMockReferenceConfig("org.apache.dubbo.config.utils.service.FooService", "group1", "1.0.0");
         assertEquals(0L, config.getCounter());
-        cache.get(config);
+        Object proxy = cache.get(config);
         assertTrue(config.isGetMethodRun());
 
+        // singleton reference config by default
         MockReferenceConfig configCopy = buildMockReferenceConfig("org.apache.dubbo.config.utils.service.FooService", "group1", "1.0.0");
         assertEquals(1L, configCopy.getCounter());
-        cache.get(configCopy);
-        assertTrue(configCopy.isGetMethodRun());
+        Object proxyOfCopyConfig = cache.get(configCopy);
+        assertFalse(configCopy.isGetMethodRun());
 
-        assertEquals(2L, config.getCounter());
-        assertEquals(2L, configCopy.getCounter());
+        assertEquals(1L, config.getCounter());
+        assertEquals(1L, configCopy.getCounter());
+        assertEquals(proxy, proxyOfCopyConfig);
     }
 
     @Test