Merge pull request apache#2725, problems of graceful shutdown in 2.6.…

…3 and some recommendation.
CrazyHZM · Dec 6, 2018 · 228efb9 · 228efb9
1 parent ac7fc9d
commit 228efb9
Show file tree

Hide file tree

Showing 20 changed files with 65 additions and 567 deletions.
diff --git a/dubbo-all/pom.xml b/dubbo-all/pom.xml
@@ -325,13 +325,6 @@
             <scope>compile</scope>
             <optional>true</optional>
         </dependency>
-        <dependency>
-            <groupId>org.apache.dubbo</groupId>
-            <artifactId>dubbo-bootstrap</artifactId>
-            <version>${project.version}</version>
-            <scope>compile</scope>
-            <optional>true</optional>
-        </dependency>
         <dependency>
             <groupId>org.apache.dubbo</groupId>
             <artifactId>dubbo-compatible</artifactId>
@@ -439,7 +432,6 @@
                                     <include>org.apache.dubbo:dubbo-serialization-kryo</include>
                                     <include>org.apache.dubbo:dubbo-serialization-jdk</include>
                                     <include>org.apache.dubbo:dubbo-serialization-protostuff</include>
-                                    <include>org.apache.dubbo:dubbo-bootstrap</include>
                                 </includes>
                             </artifactSet>
                             <transformers>

diff --git a/dubbo-bom/pom.xml b/dubbo-bom/pom.xml
@@ -283,11 +283,6 @@
                 <artifactId>dubbo-serialization-protostuff</artifactId>
                 <version>${project.version}</version>
             </dependency>
-            <dependency>
-                <groupId>org.apache.dubbo</groupId>
-                <artifactId>dubbo-bootstrap</artifactId>
-                <version>${project.version}</version>
-            </dependency>
             <dependency>
                 <groupId>org.apache.dubbo</groupId>
                 <artifactId>dubbo-compatible</artifactId>

diff --git a/dubbo-bootstrap/pom.xml b/dubbo-bootstrap/pom.xml
diff --git a/dubbo-bootstrap/src/main/java/org/apache/dubbo/bootstrap/DubboBootstrap.java b/dubbo-bootstrap/src/main/java/org/apache/dubbo/bootstrap/DubboBootstrap.java
diff --git a/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ProtocolConfig.java b/dubbo-config/dubbo-config-api/src/main/java/org/apache/dubbo/config/ProtocolConfig.java
@@ -470,13 +470,4 @@ public void destroy() {
             ExtensionLoader.getExtensionLoader(Protocol.class).getExtension(name).destroy();
         }
     }
-
-    /**
-     * Just for compatibility.
-     * It should be deleted in the next major version, say 2.7.x.
-     */
-    @Deprecated
-    public static void destroyAll() {
-        DubboShutdownHook.getDubboShutdownHook().destroyAll();
-    }
-}
+}
diff --git a/dubbo-config/dubbo-config-spring/pom.xml b/dubbo-config/dubbo-config-spring/pom.xml
@@ -34,11 +34,6 @@
             <artifactId>dubbo-config-api</artifactId>
             <version>${project.parent.version}</version>
         </dependency>
-        <dependency>
-            <groupId>org.apache.dubbo</groupId>
-            <artifactId>dubbo-bootstrap</artifactId>
-            <version>${project.parent.version}</version>
-        </dependency>
         <dependency>
             <groupId>org.springframework</groupId>
             <artifactId>spring-beans</artifactId>
@@ -157,4 +152,4 @@
             -->
         </plugins>
     </build>
-</project>
+</project>
diff --git a/...nfig/dubbo-config-spring/src/main/java/org/apache/dubbo/config/spring/AnnotationBean.java b/...nfig/dubbo-config-spring/src/main/java/org/apache/dubbo/config/spring/AnnotationBean.java
@@ -28,7 +28,6 @@
 import org.apache.dubbo.config.MonitorConfig;
 import org.apache.dubbo.config.ProtocolConfig;
 import org.apache.dubbo.config.ProviderConfig;
-import org.apache.dubbo.config.ReferenceConfig;
 import org.apache.dubbo.config.RegistryConfig;
 import org.apache.dubbo.config.ServiceConfig;
 import org.apache.dubbo.config.annotation.Reference;
@@ -111,17 +110,16 @@ public void postProcessBeanFactory(ConfigurableListableBeanFactory beanFactory)
 
     @Override
     public void destroy() {
-
-        //  This will only be called for singleton scope bean, and expected to be called by spring shutdown hook when BeanFactory/ApplicationContext destroys.
-        //  We will guarantee dubbo related resources being released with dubbo shutdown hook.
-
-        //  for (ServiceConfig<?> serviceConfig : serviceConfigs) {
-        //      try {
-        //          serviceConfig.unexport();
-        //      } catch (Throwable e) {
-        //          logger.error(e.getMessage(), e);
-        //      }
-        //  }
+        // no need to destroy here
+        // see org.apache.dubbo.config.spring.extension.SpringExtensionFactory.ShutdownHookListener
+        /*
+          for (ServiceConfig<?> serviceConfig : serviceConfigs) {
+              try {
+                  serviceConfig.unexport();
+              } catch (Throwable e) {
+                  logger.error(e.getMessage(), e);
+              }
+          }
 
         for (ReferenceConfig<?> referenceConfig : referenceConfigs.values()) {
             try {
@@ -130,6 +128,7 @@ public void destroy() {
                 logger.error(e.getMessage(), e);
             }
         }
+        */
     }
 
     @Override

diff --git a/...-config/dubbo-config-spring/src/main/java/org/apache/dubbo/config/spring/ServiceBean.java b/...-config/dubbo-config-spring/src/main/java/org/apache/dubbo/config/spring/ServiceBean.java
@@ -34,13 +34,13 @@
 import org.springframework.context.ApplicationContextAware;
 import org.springframework.context.ApplicationListener;
 import org.springframework.context.event.ContextRefreshedEvent;
-import org.springframework.context.support.AbstractApplicationContext;
 
-import java.lang.reflect.Method;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.Map;
 
+import static org.apache.dubbo.config.spring.util.BeanFactoryUtils.addApplicationListener;
+
 /**
  * ServiceFactoryBean
  *
@@ -72,23 +72,7 @@ public ServiceBean(Service service) {
     public void setApplicationContext(ApplicationContext applicationContext) {
         this.applicationContext = applicationContext;
         SpringExtensionFactory.addApplicationContext(applicationContext);
-        try {
-            Method method = applicationContext.getClass().getMethod("addApplicationListener", ApplicationListener.class); // backward compatibility to spring 2.0.1
-            method.invoke(applicationContext, this);
-            supportedApplicationListener = true;
-        } catch (Throwable t) {
-            if (applicationContext instanceof AbstractApplicationContext) {
-                try {
-                    Method method = AbstractApplicationContext.class.getDeclaredMethod("addListener", ApplicationListener.class); // backward compatibility to spring 2.0.1
-                    if (!method.isAccessible()) {
-                        method.setAccessible(true);
-                    }
-                    method.invoke(applicationContext, this);
-                    supportedApplicationListener = true;
-                } catch (Throwable t2) {
-                }
-            }
-        }
+        supportedApplicationListener = addApplicationListener(applicationContext, this);
     }
 
     @Override
@@ -249,9 +233,7 @@ && getInterface() != null && getInterface().length() > 0
 
     @Override
     public void destroy() throws Exception {
-        // This will only be called for singleton scope bean, and expected to be called by spring shutdown hook when BeanFactory/ApplicationContext destroys.
-        // We will guarantee dubbo related resources being released with dubbo shutdown hook.
-        //unexport();
+        // no need to export here, see org.apache.dubbo.config.spring.extension.SpringExtensionFactory.ShutdownHookListener
     }
 
     // merged from dubbox

diff --git a/...spring/src/main/java/org/apache/dubbo/config/spring/extension/SpringExtensionFactory.java b/...spring/src/main/java/org/apache/dubbo/config/spring/extension/SpringExtensionFactory.java
@@ -21,9 +21,14 @@
 import org.apache.dubbo.common.logger.Logger;
 import org.apache.dubbo.common.logger.LoggerFactory;
 import org.apache.dubbo.common.utils.ConcurrentHashSet;
+import org.apache.dubbo.config.DubboShutdownHook;
+import org.apache.dubbo.config.spring.util.BeanFactoryUtils;
 import org.springframework.beans.factory.NoSuchBeanDefinitionException;
 import org.springframework.beans.factory.NoUniqueBeanDefinitionException;
 import org.springframework.context.ApplicationContext;
+import org.springframework.context.ApplicationEvent;
+import org.springframework.context.ApplicationListener;
+import org.springframework.context.event.ContextClosedEvent;
 
 import java.util.Set;
 
@@ -34,9 +39,11 @@ public class SpringExtensionFactory implements ExtensionFactory {
     private static final Logger logger = LoggerFactory.getLogger(SpringExtensionFactory.class);
 
     private static final Set<ApplicationContext> contexts = new ConcurrentHashSet<ApplicationContext>();
+    private static final ApplicationListener shutdownHookListener = new ShutdownHookListener();
 
     public static void addApplicationContext(ApplicationContext context) {
         contexts.add(context);
+        BeanFactoryUtils.addApplicationListener(context, shutdownHookListener);
     }
 
     public static void removeApplicationContext(ApplicationContext context) {
@@ -93,4 +100,16 @@ public <T> T getExtension(Class<T> type, String name) {
         return null;
     }
 
+    private static class ShutdownHookListener implements ApplicationListener {
+        @Override
+        public void onApplicationEvent(ApplicationEvent event) {
+            if (event instanceof ContextClosedEvent) {
+                // we call it anyway since dubbo shutdown hook make sure its destroyAll() is re-entrant.
+                // pls. note we should not remove dubbo shutdown hook when spring framework is present, this is because
+                // its shutdown hook may not be installed.
+                DubboShutdownHook shutdownHook = DubboShutdownHook.getDubboShutdownHook();
+                shutdownHook.destroyAll();
+            }
+        }
+    }
 }