From 0bfd54cea412fef92e0ec36b2dc18136c4c3d4e0 Mon Sep 17 00:00:00 2001
From: Albumen Kevin <jhq0812@gmail.com>
Date: Fri, 1 Sep 2023 13:03:33 +0800
Subject: [PATCH] Cleanup and throw exception if bind failed (#12987)

---
 .../qos/protocol/QosProtocolWrapper.java      |  7 +--
 .../dubbo/qos/server/QosBindException.java    | 26 ++++++++++
 .../org/apache/dubbo/qos/server/Server.java   | 19 ++++----
 .../org/apache/dubbo/remoting/Constants.java  |  4 ++
 .../netty4/NettyPortUnificationServer.java    | 47 +++++++++++--------
 .../transport/netty4/NettyServer.java         | 30 +++++++-----
 .../transport/netty4/ConnectionTest.java      |  6 +--
 .../netty4/PortUnificationServerTest.java     |  5 +-
 ...ultiplexProtocolConnectionManagerTest.java |  7 ++-
 .../SingleProtocolConnectionManagerTest.java  |  3 +-
 10 files changed, 99 insertions(+), 55 deletions(-)
 create mode 100644 dubbo-plugin/dubbo-qos/src/main/java/org/apache/dubbo/qos/server/QosBindException.java

diff --git a/dubbo-plugin/dubbo-qos/src/main/java/org/apache/dubbo/qos/protocol/QosProtocolWrapper.java b/dubbo-plugin/dubbo-qos/src/main/java/org/apache/dubbo/qos/protocol/QosProtocolWrapper.java
index d45ee69749..a8fa9ca754 100644
--- a/dubbo-plugin/dubbo-qos/src/main/java/org/apache/dubbo/qos/protocol/QosProtocolWrapper.java
+++ b/dubbo-plugin/dubbo-qos/src/main/java/org/apache/dubbo/qos/protocol/QosProtocolWrapper.java
@@ -96,7 +96,9 @@ public class QosProtocolWrapper implements Protocol, ScopeModelAware {
         return protocol.getServers();
     }
 
-    private void startQosServer(URL url) {
+    private void startQosServer(URL url) throws RpcException {
+        boolean qosCheck = url.getParameter(QOS_CHECK, false);
+
         try {
             if (!hasStarted.compareAndSet(false, true)) {
                 return;
@@ -132,9 +134,8 @@ public class QosProtocolWrapper implements Protocol, ScopeModelAware {
 
         } catch (Throwable throwable) {
             logger.warn(QOS_FAILED_START_SERVER, "", "", "Fail to start qos server: ", throwable);
-            boolean qosCheck = url.getParameter(QOS_CHECK, false);
             if (qosCheck) {
-                throw new IllegalStateException("Fail to start qos server: " + throwable.getMessage(), throwable);
+                throw new RpcException(throwable);
             }
         }
     }
diff --git a/dubbo-plugin/dubbo-qos/src/main/java/org/apache/dubbo/qos/server/QosBindException.java b/dubbo-plugin/dubbo-qos/src/main/java/org/apache/dubbo/qos/server/QosBindException.java
new file mode 100644
index 0000000000..57aecd228f
--- /dev/null
+++ b/dubbo-plugin/dubbo-qos/src/main/java/org/apache/dubbo/qos/server/QosBindException.java
@@ -0,0 +1,26 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.dubbo.qos.server;
+
+/**
+ * Indicate that if Qos Start failed
+ */
+public class QosBindException extends RuntimeException {
+    public QosBindException(String message, Throwable cause) {
+        super(message, cause);
+    }
+}
diff --git a/dubbo-plugin/dubbo-qos/src/main/java/org/apache/dubbo/qos/server/Server.java b/dubbo-plugin/dubbo-qos/src/main/java/org/apache/dubbo/qos/server/Server.java
index 65e7e3b9fa..8bac2d8ab9 100644
--- a/dubbo-plugin/dubbo-qos/src/main/java/org/apache/dubbo/qos/server/Server.java
+++ b/dubbo-plugin/dubbo-qos/src/main/java/org/apache/dubbo/qos/server/Server.java
@@ -35,8 +35,6 @@ import io.netty.util.concurrent.DefaultThreadFactory;
 
 import java.util.concurrent.atomic.AtomicBoolean;
 
-import static org.apache.dubbo.common.constants.LoggerCodeConstants.QOS_FAILED_START_SERVER;
-
 /**
  * A server serves for both telnet access and http access
  * <ul>
@@ -105,13 +103,13 @@ public class Server {
             @Override
             protected void initChannel(Channel ch) throws Exception {
                 ch.pipeline().addLast(new QosProcessHandler(frameworkModel,
-                    QosConfiguration.builder()
-                        .welcome(welcome)
-                        .acceptForeignIp(acceptForeignIp)
-                        .acceptForeignIpWhitelist(acceptForeignIpWhitelist)
-                        .anonymousAccessPermissionLevel(anonymousAccessPermissionLevel)
-                        .anonymousAllowCommands(anonymousAllowCommands)
-                        .build()
+                        QosConfiguration.builder()
+                                .welcome(welcome)
+                                .acceptForeignIp(acceptForeignIp)
+                                .acceptForeignIpWhitelist(acceptForeignIpWhitelist)
+                                .anonymousAccessPermissionLevel(anonymousAccessPermissionLevel)
+                                .anonymousAllowCommands(anonymousAllowCommands)
+                                .build()
                 ));
             }
         });
@@ -124,8 +122,7 @@ public class Server {
 
             logger.info("qos-server bind localhost:" + port);
         } catch (Throwable throwable) {
-            logger.error(QOS_FAILED_START_SERVER, "", "", "qos-server can not bind localhost:" + port, throwable);
-            throw throwable;
+            throw new QosBindException("qos-server can not bind localhost:" + port, throwable);
         }
     }
 
diff --git a/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/Constants.java b/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/Constants.java
index 51b42ec185..ae7ab3f682 100644
--- a/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/Constants.java
+++ b/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/Constants.java
@@ -127,6 +127,10 @@ public interface Constants {
 
     String BIND_PORT_KEY = "bind.port";
 
+    String BIND_RETRY_TIMES = "bind.retry.times";
+
+    String BIND_RETRY_INTERVAL = "bind.retry.interval";
+
     String SENT_KEY = "sent";
 
     String DISPATCHER_KEY = "dispatcher";
diff --git a/dubbo-remoting/dubbo-remoting-netty4/src/main/java/org/apache/dubbo/remoting/transport/netty4/NettyPortUnificationServer.java b/dubbo-remoting/dubbo-remoting-netty4/src/main/java/org/apache/dubbo/remoting/transport/netty4/NettyPortUnificationServer.java
index f5fdc9a919..a7cab7b985 100644
--- a/dubbo-remoting/dubbo-remoting-netty4/src/main/java/org/apache/dubbo/remoting/transport/netty4/NettyPortUnificationServer.java
+++ b/dubbo-remoting/dubbo-remoting-netty4/src/main/java/org/apache/dubbo/remoting/transport/netty4/NettyPortUnificationServer.java
@@ -97,14 +97,14 @@ public class NettyPortUnificationServer extends AbstractPortUnificationServer {
         }
     }
 
-    public void bind() {
+    public void bind() throws Throwable {
         if (channel == null) {
             doOpen();
         }
     }
 
     @Override
-    public void doOpen() {
+    public void doOpen() throws Throwable {
         bootstrap = new ServerBootstrap();
 
         bossGroup = NettyEventLoopFactory.eventLoopGroup(1, EVENT_LOOP_BOSS_POOL_NAME);
@@ -138,9 +138,31 @@ public class NettyPortUnificationServer extends AbstractPortUnificationServer {
             bindIp = ANYHOST_VALUE;
         }
         InetSocketAddress bindAddress = new InetSocketAddress(bindIp, bindPort);
-        ChannelFuture channelFuture = bootstrap.bind(bindAddress);
-        channelFuture.syncUninterruptibly();
-        channel = channelFuture.channel();
+        try {
+            ChannelFuture channelFuture = bootstrap.bind(bindAddress);
+            channelFuture.syncUninterruptibly();
+            channel = channelFuture.channel();
+        } catch (Throwable t) {
+            closeBootstrap();
+            throw t;
+        }
+    }
+
+    private void closeBootstrap() {
+        try {
+            if (bootstrap != null) {
+                long timeout = ConfigurationUtils.reCalShutdownTime(serverShutdownTimeoutMills);
+                long quietPeriod = Math.min(2000L, timeout);
+                Future<?> bossGroupShutdownFuture = bossGroup.shutdownGracefully(quietPeriod,
+                    timeout, MILLISECONDS);
+                Future<?> workerGroupShutdownFuture = workerGroup.shutdownGracefully(quietPeriod,
+                    timeout, MILLISECONDS);
+                bossGroupShutdownFuture.awaitUninterruptibly(timeout, MILLISECONDS);
+                workerGroupShutdownFuture.awaitUninterruptibly(timeout, MILLISECONDS);
+            }
+        } catch (Throwable e) {
+            logger.warn(TRANSPORT_FAILED_CLOSE, "", "", e.getMessage(), e);
+        }
     }
 
     @Override
@@ -176,20 +198,7 @@ public class NettyPortUnificationServer extends AbstractPortUnificationServer {
             protocol.close();
         }
 
-        try {
-            if (bootstrap != null) {
-                long timeout = ConfigurationUtils.reCalShutdownTime(serverShutdownTimeoutMills);
-                long quietPeriod = Math.min(2000L, timeout);
-                Future<?> bossGroupShutdownFuture = bossGroup.shutdownGracefully(quietPeriod,
-                    timeout, MILLISECONDS);
-                Future<?> workerGroupShutdownFuture = workerGroup.shutdownGracefully(quietPeriod,
-                    timeout, MILLISECONDS);
-                bossGroupShutdownFuture.awaitUninterruptibly(timeout, MILLISECONDS);
-                workerGroupShutdownFuture.awaitUninterruptibly(timeout, MILLISECONDS);
-            }
-        } catch (Throwable e) {
-            logger.warn(TRANSPORT_FAILED_CLOSE, "", "", e.getMessage(), e);
-        }
+        closeBootstrap();
     }
 
     @Override
diff --git a/dubbo-remoting/dubbo-remoting-netty4/src/main/java/org/apache/dubbo/remoting/transport/netty4/NettyServer.java b/dubbo-remoting/dubbo-remoting-netty4/src/main/java/org/apache/dubbo/remoting/transport/netty4/NettyServer.java
index 8511370b5d..1c0c7da3a9 100644
--- a/dubbo-remoting/dubbo-remoting-netty4/src/main/java/org/apache/dubbo/remoting/transport/netty4/NettyServer.java
+++ b/dubbo-remoting/dubbo-remoting-netty4/src/main/java/org/apache/dubbo/remoting/transport/netty4/NettyServer.java
@@ -105,10 +105,14 @@ public class NettyServer extends AbstractServer {
         initServerBootstrap(nettyServerHandler);
 
         // bind
-        ChannelFuture channelFuture = bootstrap.bind(getBindAddress());
-        channelFuture.syncUninterruptibly();
-        channel = channelFuture.channel();
-
+        try {
+            ChannelFuture channelFuture = bootstrap.bind(getBindAddress());
+            channelFuture.syncUninterruptibly();
+            channel = channelFuture.channel();
+        } catch (Throwable t) {
+            closeBootstrap();
+            throw t;
+        }
     }
 
     protected EventLoopGroup createBossGroup() {
@@ -172,6 +176,17 @@ public class NettyServer extends AbstractServer {
         } catch (Throwable e) {
             logger.warn(TRANSPORT_FAILED_CLOSE, "", "", e.getMessage(), e);
         }
+        closeBootstrap();
+        try {
+            if (channels != null) {
+                channels.clear();
+            }
+        } catch (Throwable e) {
+            logger.warn(TRANSPORT_FAILED_CLOSE, "", "", e.getMessage(), e);
+        }
+    }
+
+    private void closeBootstrap() {
         try {
             if (bootstrap != null) {
                 long timeout = ConfigurationUtils.reCalShutdownTime(serverShutdownTimeoutMills);
@@ -184,13 +199,6 @@ public class NettyServer extends AbstractServer {
         } catch (Throwable e) {
             logger.warn(TRANSPORT_FAILED_CLOSE, "", "", e.getMessage(), e);
         }
-        try {
-            if (channels != null) {
-                channels.clear();
-            }
-        } catch (Throwable e) {
-            logger.warn(TRANSPORT_FAILED_CLOSE, "", "", e.getMessage(), e);
-        }
     }
 
     @Override
diff --git a/dubbo-remoting/dubbo-remoting-netty4/src/test/java/org/apache/dubbo/remoting/transport/netty4/ConnectionTest.java b/dubbo-remoting/dubbo-remoting-netty4/src/test/java/org/apache/dubbo/remoting/transport/netty4/ConnectionTest.java
index 2c230d6c61..ba9fd3eb0d 100644
--- a/dubbo-remoting/dubbo-remoting-netty4/src/test/java/org/apache/dubbo/remoting/transport/netty4/ConnectionTest.java
+++ b/dubbo-remoting/dubbo-remoting-netty4/src/test/java/org/apache/dubbo/remoting/transport/netty4/ConnectionTest.java
@@ -21,13 +21,13 @@ import org.apache.dubbo.common.constants.CommonConstants;
 import org.apache.dubbo.common.utils.NetUtils;
 import org.apache.dubbo.config.ApplicationConfig;
 import org.apache.dubbo.config.context.ConfigManager;
-import org.apache.dubbo.remoting.RemotingException;
 import org.apache.dubbo.remoting.api.connection.AbstractConnectionClient;
 import org.apache.dubbo.remoting.api.connection.ConnectionManager;
 import org.apache.dubbo.remoting.api.connection.MultiplexProtocolConnectionManager;
 import org.apache.dubbo.remoting.api.pu.DefaultPuHandler;
 import org.apache.dubbo.rpc.model.ApplicationModel;
 import org.apache.dubbo.rpc.model.ModuleModel;
+
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.BeforeAll;
@@ -50,7 +50,7 @@ public class ConnectionTest {
     private static ConnectionManager connectionManager;
 
     @BeforeAll
-    public static void init() throws RemotingException {
+    public static void init() throws Throwable {
         int port = NetUtils.getAvailablePort();
         url = URL.valueOf("empty://127.0.0.1:" + port + "?foo=bar");
         ApplicationModel applicationModel = ApplicationModel.defaultModel();
@@ -123,7 +123,7 @@ public class ConnectionTest {
     }
 
     @Test
-    void connectSyncTest() throws RemotingException {
+    void connectSyncTest() throws Throwable {
         int port = NetUtils.getAvailablePort();
         URL url = URL.valueOf("empty://127.0.0.1:" + port + "?foo=bar");
         NettyPortUnificationServer nettyPortUnificationServer = new NettyPortUnificationServer(url, new DefaultPuHandler());
diff --git a/dubbo-remoting/dubbo-remoting-netty4/src/test/java/org/apache/dubbo/remoting/transport/netty4/PortUnificationServerTest.java b/dubbo-remoting/dubbo-remoting-netty4/src/test/java/org/apache/dubbo/remoting/transport/netty4/PortUnificationServerTest.java
index 0f5268da89..c5b1234e0e 100644
--- a/dubbo-remoting/dubbo-remoting-netty4/src/test/java/org/apache/dubbo/remoting/transport/netty4/PortUnificationServerTest.java
+++ b/dubbo-remoting/dubbo-remoting-netty4/src/test/java/org/apache/dubbo/remoting/transport/netty4/PortUnificationServerTest.java
@@ -21,11 +21,10 @@ import org.apache.dubbo.common.constants.CommonConstants;
 import org.apache.dubbo.common.utils.NetUtils;
 import org.apache.dubbo.config.ApplicationConfig;
 import org.apache.dubbo.config.context.ConfigManager;
-import org.apache.dubbo.remoting.RemotingException;
 import org.apache.dubbo.remoting.api.pu.DefaultPuHandler;
-
 import org.apache.dubbo.rpc.model.ApplicationModel;
 import org.apache.dubbo.rpc.model.ModuleModel;
+
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 
@@ -34,7 +33,7 @@ import static org.apache.dubbo.common.constants.CommonConstants.EXECUTOR_MANAGEM
 class PortUnificationServerTest {
 
     @Test
-    void testBind() throws RemotingException {
+    void testBind() throws Throwable {
         int port = NetUtils.getAvailablePort();
         URL url = URL.valueOf("empty://127.0.0.1:" + port + "?foo=bar");
         ApplicationModel applicationModel = ApplicationModel.defaultModel();
diff --git a/dubbo-remoting/dubbo-remoting-netty4/src/test/java/org/apache/dubbo/remoting/transport/netty4/api/MultiplexProtocolConnectionManagerTest.java b/dubbo-remoting/dubbo-remoting-netty4/src/test/java/org/apache/dubbo/remoting/transport/netty4/api/MultiplexProtocolConnectionManagerTest.java
index 2bceec7ab6..76a45adadd 100644
--- a/dubbo-remoting/dubbo-remoting-netty4/src/test/java/org/apache/dubbo/remoting/transport/netty4/api/MultiplexProtocolConnectionManagerTest.java
+++ b/dubbo-remoting/dubbo-remoting-netty4/src/test/java/org/apache/dubbo/remoting/transport/netty4/api/MultiplexProtocolConnectionManagerTest.java
@@ -21,15 +21,14 @@ import org.apache.dubbo.common.URL;
 import org.apache.dubbo.common.constants.CommonConstants;
 import org.apache.dubbo.config.ApplicationConfig;
 import org.apache.dubbo.config.context.ConfigManager;
-import org.apache.dubbo.remoting.RemotingException;
 import org.apache.dubbo.remoting.api.connection.AbstractConnectionClient;
 import org.apache.dubbo.remoting.api.connection.ConnectionManager;
 import org.apache.dubbo.remoting.api.connection.MultiplexProtocolConnectionManager;
 import org.apache.dubbo.remoting.api.pu.DefaultPuHandler;
 import org.apache.dubbo.remoting.transport.netty4.NettyPortUnificationServer;
-
 import org.apache.dubbo.rpc.model.ApplicationModel;
 import org.apache.dubbo.rpc.model.ModuleModel;
+
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.BeforeAll;
@@ -52,7 +51,7 @@ public class MultiplexProtocolConnectionManagerTest {
     private static ConnectionManager connectionManager;
 
     @BeforeAll
-    public static void init() throws RemotingException {
+    public static void init() throws Throwable {
         ApplicationModel applicationModel = ApplicationModel.defaultModel();
         ApplicationConfig applicationConfig = new ApplicationConfig("provider-app");
         applicationConfig.setExecutorManagementMode(EXECUTOR_MANAGEMENT_MODE_DEFAULT);
@@ -95,7 +94,7 @@ public class MultiplexProtocolConnectionManagerTest {
     }
 
     @Test
-    public void testForEachConnection() throws RemotingException {
+    public void testForEachConnection() throws Throwable {
         DefaultPuHandler handler = new DefaultPuHandler();
 
         NettyPortUnificationServer server2 = new NettyPortUnificationServer(url2, handler);
diff --git a/dubbo-remoting/dubbo-remoting-netty4/src/test/java/org/apache/dubbo/remoting/transport/netty4/api/SingleProtocolConnectionManagerTest.java b/dubbo-remoting/dubbo-remoting-netty4/src/test/java/org/apache/dubbo/remoting/transport/netty4/api/SingleProtocolConnectionManagerTest.java
index bc4d548008..5369ea0f22 100644
--- a/dubbo-remoting/dubbo-remoting-netty4/src/test/java/org/apache/dubbo/remoting/transport/netty4/api/SingleProtocolConnectionManagerTest.java
+++ b/dubbo-remoting/dubbo-remoting-netty4/src/test/java/org/apache/dubbo/remoting/transport/netty4/api/SingleProtocolConnectionManagerTest.java
@@ -31,6 +31,7 @@ import org.apache.dubbo.remoting.transport.netty4.NettyConnectionClient;
 import org.apache.dubbo.remoting.transport.netty4.NettyPortUnificationServer;
 import org.apache.dubbo.rpc.model.ApplicationModel;
 import org.apache.dubbo.rpc.model.ModuleModel;
+
 import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.BeforeAll;
@@ -51,7 +52,7 @@ public class SingleProtocolConnectionManagerTest {
     private static ConnectionManager connectionManager;
 
     @BeforeAll
-    public static void init() throws RemotingException {
+    public static void init() throws Throwable {
         int port = NetUtils.getAvailablePort();
         url = URL.valueOf("empty://127.0.0.1:" + port + "?foo=bar");
         ApplicationModel applicationModel = ApplicationModel.defaultModel();