[GitHub] liubao68 closed pull request #761: [SCB-658] MicroserviceVersions eventbus leak

[GitBox <gi...@apache.org>]

liubao68 closed pull request #761: [SCB-658] MicroserviceVersions eventbus leak
URL: https://github.com/apache/incubator-servicecomb-java-chassis/pull/761
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/core/src/main/java/org/apache/servicecomb/core/filter/OperationInstancesDiscoveryFilter.java b/core/src/main/java/org/apache/servicecomb/core/filter/OperationInstancesDiscoveryFilter.java
index c0959bd07..cd7a6ff0c 100644
--- a/core/src/main/java/org/apache/servicecomb/core/filter/OperationInstancesDiscoveryFilter.java
+++ b/core/src/main/java/org/apache/servicecomb/core/filter/OperationInstancesDiscoveryFilter.java
@@ -113,9 +113,8 @@ protected void fillInstances(Map<String, DiscoveryTreeNode> operationNodes,
     Map<MicroserviceVersionMeta, Map<String, MicroserviceInstance>> versionMap = new IdentityHashMap<>();
     for (MicroserviceInstance instance : instances.values()) {
       MicroserviceVersionMeta versionMeta = MicroserviceVersions.getVersion(instance.getServiceId());
-      Map<String, MicroserviceInstance> versionInstances = versionMap.computeIfAbsent(versionMeta, vm -> {
-        return new HashMap<>();
-      });
+      Map<String, MicroserviceInstance> versionInstances = versionMap
+          .computeIfAbsent(versionMeta, vm -> new HashMap<>());
       versionInstances.put(instance.getInstanceId(), instance);
     }
     return versionMap;
diff --git a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/consumer/AppManager.java b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/consumer/AppManager.java
index 44fcb9deb..8093ceb5e 100644
--- a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/consumer/AppManager.java
+++ b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/consumer/AppManager.java
@@ -56,9 +56,7 @@ public MicroserviceVersionRule getOrCreateMicroserviceVersionRule(String appId,
   }
 
   public MicroserviceManager getOrCreateMicroserviceManager(String appId) {
-    return apps.computeIfAbsent(appId, id -> {
-      return new MicroserviceManager(this, appId);
-    });
+    return apps.computeIfAbsent(appId, id -> new MicroserviceManager(this, appId));
   }
 
   public MicroserviceVersions getOrCreateMicroserviceVersions(String appId, String microserviceName) {
diff --git a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/consumer/MicroserviceManager.java b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/consumer/MicroserviceManager.java
index 3181fde2b..ab3d9ca6a 100644
--- a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/consumer/MicroserviceManager.java
+++ b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/consumer/MicroserviceManager.java
@@ -49,6 +49,7 @@ public MicroserviceVersions getOrCreateMicroserviceVersions(String microserviceN
     if (!microserviceVersions.isValidated()) {
       // remove this microservice if it does not exist or not registered in order to get it back when access it again
       versionsByName.remove(microserviceName);
+      appManager.getEventBus().unregister(microserviceVersions);
     }
     return microserviceVersions;
   }
diff --git a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/discovery/AbstractEndpointDiscoveryFilter.java b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/discovery/AbstractEndpointDiscoveryFilter.java
index 8c1654f42..dd471af8e 100644
--- a/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/discovery/AbstractEndpointDiscoveryFilter.java
+++ b/service-registry/src/main/java/org/apache/servicecomb/serviceregistry/discovery/AbstractEndpointDiscoveryFilter.java
@@ -39,9 +39,8 @@ public boolean isGroupingFilter() {
   @Override
   public DiscoveryTreeNode discovery(DiscoveryContext context, DiscoveryTreeNode parent) {
     String expectTransportName = findTransportName(context, parent);
-    return parent.children().computeIfAbsent(expectTransportName, etn -> {
-      return createDiscoveryTreeNode(expectTransportName, context, parent);
-    });
+    return parent.children()
+        .computeIfAbsent(expectTransportName, etn -> createDiscoveryTreeNode(expectTransportName, context, parent));
   }
 
   @SuppressWarnings("unchecked")
diff --git a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/consumer/TestMicroserviceManager.java b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/consumer/TestMicroserviceManager.java
index 8a5f23667..5ea41252a 100644
--- a/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/consumer/TestMicroserviceManager.java
+++ b/service-registry/src/test/java/org/apache/servicecomb/serviceregistry/consumer/TestMicroserviceManager.java
@@ -19,11 +19,13 @@
 
 import java.util.Collections;
 import java.util.Map;
+import java.util.concurrent.atomic.AtomicInteger;
 
 import javax.xml.ws.Holder;
 
 import org.apache.servicecomb.serviceregistry.RegistryUtils;
 import org.apache.servicecomb.serviceregistry.api.response.FindInstancesResponse;
+import org.apache.servicecomb.serviceregistry.api.response.MicroserviceInstanceChangedEvent;
 import org.apache.servicecomb.serviceregistry.client.http.MicroserviceInstances;
 import org.apache.servicecomb.serviceregistry.definition.DefinitionConst;
 import org.apache.servicecomb.serviceregistry.task.event.PeriodicPullEvent;
@@ -98,11 +100,23 @@ public void testCreateRuleServiceNotExists() {
       }
     };
 
+    AtomicInteger triggerCount = new AtomicInteger();
+    new MockUp<MicroserviceVersions>() {
+      @Mock
+      void onMicroserviceInstanceChanged(MicroserviceInstanceChangedEvent changedEvent) {
+        triggerCount.incrementAndGet();
+      }
+    };
+
     MicroserviceVersionRule microserviceVersionRule =
         microserviceManager.getOrCreateMicroserviceVersionRule(serviceName, versionRule);
     Assert.assertEquals("0.0.0+", microserviceVersionRule.getVersionRule().getVersionRule());
     Assert.assertNull(microserviceVersionRule.getLatestMicroserviceVersion());
     Assert.assertEquals(0, cachedVersions.size());
+
+    MicroserviceInstanceChangedEvent changedEvent = new MicroserviceInstanceChangedEvent();
+    eventBus.post(changedEvent);
+    Assert.assertEquals(0, triggerCount.get());
   }
 
   @Test


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services