You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dubbo.apache.org by al...@apache.org on 2021/09/28 06:18:27 UTC

[dubbo] branch 3.0 updated: [3.0] destroy executor and improve tests 0926 (#8928)

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

albumenj 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 30d0fac  [3.0] destroy executor and improve tests 0926 (#8928)
30d0fac is described below

commit 30d0face444b1950ec2215376f8af20707f076aa
Author: Gong Dewei <ky...@qq.com>
AuthorDate: Tue Sep 28 14:18:14 2021 +0800

    [3.0] destroy executor and improve tests 0926 (#8928)
    
    * shutdown scheduledExecutors and executorServiceRing
    
    * fix NPE of configuration
    
    * Fix notify loop after executor service is shutdown
    
    * improve registry center in tests
    
    * polish unregister shutdown hook log
    
    * Fix systemConfiguration NPE
    
    * enable surefire reuseForks for fast testing
    
    * Fix checking DubboShutdownHook is alive
---
 .../org/apache/dubbo/rpc/cluster/RouterChain.java  | 22 ++++++++++++++------
 .../manager/DefaultExecutorRepository.java         | 24 ++++++++++++++--------
 .../org/apache/dubbo/config/DubboShutdownHook.java |  8 ++++++++
 dubbo-config/dubbo-config-spring/pom.xml           |  2 +-
 .../consumer3/PropertySourcesInJavaConfigTest.java | 18 ++++++++++++----
 .../javaconfig/JavaConfigReferenceBeanTest.java    | 18 ++++++++++++----
 .../rpc/protocol/dubbo/DecodeableRpcResult.java    |  4 +++-
 7 files changed, 72 insertions(+), 24 deletions(-)

diff --git a/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/RouterChain.java b/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/RouterChain.java
index c2adc63..bb10de5 100644
--- a/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/RouterChain.java
+++ b/dubbo-cluster/src/main/java/org/apache/dubbo/rpc/cluster/RouterChain.java
@@ -38,6 +38,7 @@ import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.ExecutorService;
+import java.util.concurrent.RejectedExecutionException;
 import java.util.concurrent.Semaphore;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicReference;
@@ -271,14 +272,23 @@ public class RouterChain<T> {
         if (firstBuildCache.compareAndSet(true,false)) {
             buildCache(notify);
         }
-        if (notify) {
-            if (loopPermitNotify.tryAcquire()) {
-                loopPool.submit(new NotifyLoopRunnable(true, loopPermitNotify));
+
+        try {
+            if (notify) {
+                if (loopPermitNotify.tryAcquire()) {
+                    loopPool.submit(new NotifyLoopRunnable(true, loopPermitNotify));
+                }
+            } else {
+                if (loopPermit.tryAcquire()) {
+                    loopPool.submit(new NotifyLoopRunnable(false, loopPermit));
+                }
             }
-        } else {
-            if (loopPermit.tryAcquire()) {
-                loopPool.submit(new NotifyLoopRunnable(false, loopPermit));
+        } catch (RejectedExecutionException e) {
+            if (loopPool.isShutdown()){
+                logger.warn("loopPool executor service is shutdown, ignoring notify loop");
+                return;
             }
+            throw e;
         }
     }
 
diff --git a/dubbo-common/src/main/java/org/apache/dubbo/common/threadpool/manager/DefaultExecutorRepository.java b/dubbo-common/src/main/java/org/apache/dubbo/common/threadpool/manager/DefaultExecutorRepository.java
index 739406a..e0da1e9 100644
--- a/dubbo-common/src/main/java/org/apache/dubbo/common/threadpool/manager/DefaultExecutorRepository.java
+++ b/dubbo-common/src/main/java/org/apache/dubbo/common/threadpool/manager/DefaultExecutorRepository.java
@@ -387,14 +387,22 @@ public class DefaultExecutorRepository implements ExecutorRepository, ExtensionA
             }
         });
 
-        // TODO shutdown all executor services
-//        for (ScheduledExecutorService executorService : scheduledExecutors.listItems()) {
-//            executorService.shutdown();
-//        }
-//
-//        for (ExecutorService executorService : executorServiceRing.listItems()) {
-//            executorService.shutdown();
-//        }
+        // shutdown all executor services
+        for (ScheduledExecutorService executorService : scheduledExecutors.listItems()) {
+            try {
+                executorService.shutdown();
+            } catch (Exception e) {
+                logger.warn("shutdown scheduledExecutors failed: " + e.getMessage(), e);
+            }
+        }
+
+        for (ExecutorService executorService : executorServiceRing.listItems()) {
+            try {
+                executorService.shutdown();
+            } catch (Exception e) {
+                logger.warn("shutdown executorServiceRing failed: " + e.getMessage(), e);
+            }
+        }
     }
 
     @Override
diff --git a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/DubboShutdownHook.java b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/DubboShutdownHook.java
index c8beaf0..2ecba74 100644
--- a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/DubboShutdownHook.java
+++ b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/DubboShutdownHook.java
@@ -74,6 +74,8 @@ public class DubboShutdownHook extends Thread {
         if (registered.compareAndSet(false, true)) {
             try {
                 Runtime.getRuntime().addShutdownHook(this);
+            } catch (IllegalStateException e) {
+                logger.warn("register shutdown hook failed: " + e.getMessage());
             } catch (Exception e) {
                 logger.warn("register shutdown hook failed: " + e.getMessage(), e);
             }
@@ -85,8 +87,14 @@ public class DubboShutdownHook extends Thread {
      */
     public void unregister() {
         if (registered.compareAndSet(true, false)) {
+            if (this.isAlive() || destroyed.get()) {
+                // DubboShutdownHook is running
+                return;
+            }
             try {
                 Runtime.getRuntime().removeShutdownHook(this);
+            } catch (IllegalStateException e) {
+                logger.warn("unregister shutdown hook failed: " + e.getMessage());
             } catch (Exception e) {
                 logger.warn("unregister shutdown hook failed: " + e.getMessage(), e);
             }
diff --git a/dubbo-config/dubbo-config-spring/pom.xml b/dubbo-config/dubbo-config-spring/pom.xml
index 9c8e3ea..5d607c4 100644
--- a/dubbo-config/dubbo-config-spring/pom.xml
+++ b/dubbo-config/dubbo-config-spring/pom.xml
@@ -274,7 +274,7 @@
                 <groupId>org.apache.maven.plugins</groupId>
                 <configuration>
                     <forkCount>1</forkCount>
-                    <reuseForks>false</reuseForks>
+                    <!--<reuseForks>false</reuseForks>-->
                 </configuration>
             </plugin>
         </plugins>
diff --git a/dubbo-config/dubbo-config-spring/src/test/java/org/apache/dubbo/config/spring/propertyconfigurer/consumer3/PropertySourcesInJavaConfigTest.java b/dubbo-config/dubbo-config-spring/src/test/java/org/apache/dubbo/config/spring/propertyconfigurer/consumer3/PropertySourcesInJavaConfigTest.java
index 878849d..163b466 100644
--- a/dubbo-config/dubbo-config-spring/src/test/java/org/apache/dubbo/config/spring/propertyconfigurer/consumer3/PropertySourcesInJavaConfigTest.java
+++ b/dubbo-config/dubbo-config-spring/src/test/java/org/apache/dubbo/config/spring/propertyconfigurer/consumer3/PropertySourcesInJavaConfigTest.java
@@ -22,8 +22,10 @@ import org.apache.dubbo.config.spring.context.annotation.EnableDubbo;
 import org.apache.dubbo.config.spring.propertyconfigurer.consumer.DemoBeanFactoryPostProcessor;
 import org.apache.dubbo.config.spring.registrycenter.RegistryCenter;
 import org.apache.dubbo.config.spring.registrycenter.ZookeeperSingleRegistryCenter;
+import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.springframework.context.annotation.AnnotationConfigApplicationContext;
@@ -43,19 +45,27 @@ public class PropertySourcesInJavaConfigTest {
     private static final String SCAN_PACKAGE_NAME = "org.apache.dubbo.config.spring.propertyconfigurer.consumer3.notexist";
     private static final String PACKAGE_PATH = "/org/apache/dubbo/config/spring/propertyconfigurer/consumer3";
     private static final String PROVIDER_CONFIG_PATH = "org/apache/dubbo/config/spring/propertyconfigurer/provider/dubbo-provider.xml";
-    private RegistryCenter singleRegistryCenter;
+    private static RegistryCenter singleRegistryCenter;
 
-    @BeforeEach
-    public void setUp() throws Exception {
+    @BeforeAll
+    public static void beforeAll() {
         singleRegistryCenter = new ZookeeperSingleRegistryCenter();
         singleRegistryCenter.startup();
+    }
+
+    @AfterAll
+    public static void afterAll() {
+        singleRegistryCenter.shutdown();
+    }
+
+    @BeforeEach
+    public void setUp() throws Exception {
         DubboBootstrap.reset();
     }
 
     @AfterEach
     public void tearDown() throws IOException {
         DubboBootstrap.reset();
-        singleRegistryCenter.shutdown();
     }
 
     @BeforeEach
diff --git a/dubbo-config/dubbo-config-spring/src/test/java/org/apache/dubbo/config/spring/reference/javaconfig/JavaConfigReferenceBeanTest.java b/dubbo-config/dubbo-config-spring/src/test/java/org/apache/dubbo/config/spring/reference/javaconfig/JavaConfigReferenceBeanTest.java
index e38cd7a..ad17975 100644
--- a/dubbo-config/dubbo-config-spring/src/test/java/org/apache/dubbo/config/spring/reference/javaconfig/JavaConfigReferenceBeanTest.java
+++ b/dubbo-config/dubbo-config-spring/src/test/java/org/apache/dubbo/config/spring/reference/javaconfig/JavaConfigReferenceBeanTest.java
@@ -30,8 +30,10 @@ import org.apache.dubbo.config.spring.registrycenter.RegistryCenter;
 import org.apache.dubbo.config.spring.registrycenter.ZookeeperMultipleRegistryCenter;
 import org.apache.dubbo.rpc.service.GenericException;
 import org.apache.dubbo.rpc.service.GenericService;
+import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Disabled;
 import org.junit.jupiter.api.Test;
@@ -48,18 +50,26 @@ import java.util.Map;
 
 public class JavaConfigReferenceBeanTest {
 
-    private RegistryCenter multipleRegistryCenter;
+    private static RegistryCenter multipleRegistryCenter;
+
+    @BeforeAll
+    public static void beforeAll() {
+        multipleRegistryCenter = new ZookeeperMultipleRegistryCenter();
+        multipleRegistryCenter.startup();
+    }
+
+    @AfterAll
+    public static void afterAll() {
+        multipleRegistryCenter.shutdown();
+    }
 
     @BeforeEach
     public void setUp() {
         DubboBootstrap.reset();
-        multipleRegistryCenter = new ZookeeperMultipleRegistryCenter();
-        multipleRegistryCenter.startup();
     }
 
     @AfterEach
     public void tearDown() {
-        multipleRegistryCenter.shutdown();
         DubboBootstrap.reset();
     }
 
diff --git a/dubbo-rpc/dubbo-rpc-dubbo/src/main/java/org/apache/dubbo/rpc/protocol/dubbo/DecodeableRpcResult.java b/dubbo-rpc/dubbo-rpc-dubbo/src/main/java/org/apache/dubbo/rpc/protocol/dubbo/DecodeableRpcResult.java
index 8791e28..6d1a9d0 100644
--- a/dubbo-rpc/dubbo-rpc-dubbo/src/main/java/org/apache/dubbo/rpc/protocol/dubbo/DecodeableRpcResult.java
+++ b/dubbo-rpc/dubbo-rpc-dubbo/src/main/java/org/apache/dubbo/rpc/protocol/dubbo/DecodeableRpcResult.java
@@ -16,6 +16,7 @@
  */
 package org.apache.dubbo.rpc.protocol.dubbo;
 
+import org.apache.dubbo.common.config.Configuration;
 import org.apache.dubbo.common.config.ConfigurationUtils;
 import org.apache.dubbo.common.logger.Logger;
 import org.apache.dubbo.common.logger.LoggerFactory;
@@ -123,7 +124,8 @@ public class DecodeableRpcResult extends AppResponse implements Codec, Decodeabl
         if (!hasDecoded && channel != null && inputStream != null) {
             try {
                 if (invocation != null) {
-                    if (ConfigurationUtils.getSystemConfiguration(channel.getUrl().getScopeModel()).getBoolean(SERIALIZATION_SECURITY_CHECK_KEY, true)) {
+                    Configuration systemConfiguration = ConfigurationUtils.getSystemConfiguration(channel.getUrl().getScopeModel());
+                    if (systemConfiguration == null || systemConfiguration.getBoolean(SERIALIZATION_SECURITY_CHECK_KEY, true)) {
                         Object serializationTypeObj = invocation.get(SERIALIZATION_ID_KEY);
                         if (serializationTypeObj != null) {
                             if ((byte) serializationTypeObj != serializationType) {