Fix uninitialized non-static final fields potential NPE issue in dubbo-remoting module (#15602)

* Fix uninitialized final variables issue in dubbo-remoting

* Remove redundant initializations of AbstractPortUnificationServer fields

* Change modifier of AbstractTimerTask start method

---------

Co-authored-by: earthchen <earthchen1996@gmail.com>

Author: zrlw

dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/api/pu/AbstractPortUnificationServer.java

@@ -44,14 +44,14 @@ public abstract class AbstractPortUnificationServer extends AbstractServer {
     protocol name --> URL object
     wire protocol will get url object to config server pipeline for channel
      */
-    private final Map<String, URL> supportedUrls = new ConcurrentHashMap<>();
+    private Map<String, URL> supportedUrls;
 
     /*
     protocol name --> ChannelHandler object
     wire protocol will get handler to config server pipeline for channel
     (for triple protocol, it's a default handler that do nothing)
      */
-    private final Map<String, ChannelHandler> supportedHandlers = new ConcurrentHashMap<>();
+    private Map<String, ChannelHandler> supportedHandlers;
 
     public AbstractPortUnificationServer(URL url, ChannelHandler handler) throws RemotingException {
         super(url, handler);
@@ -63,6 +63,10 @@ public Map<String, WireProtocol> getProtocols() {
 
     @Override
     protected final void doOpen() {
+        // initialize supportedUrls and supportedHandlers before potential usage to avoid NPE.
+        supportedUrls = new ConcurrentHashMap<>();
+        supportedHandlers = new ConcurrentHashMap<>();
+
         ExtensionLoader<WireProtocol> loader =
                 getUrl().getOrDefaultFrameworkModel().getExtensionLoader(WireProtocol.class);
         Map<String, WireProtocol> protocols = loader.getActivateExtension(getUrl(), new String[0]).stream()

dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/exchange/support/header/AbstractTimerTask.java

@@ -42,7 +42,7 @@ public abstract class AbstractTimerTask implements TimerTask {
         this.channelProvider = channelProvider;
         this.hashedWheelTimer = hashedWheelTimer;
         this.tick = tick;
-        start();
+        // do not start here because inheritor should set additional timeout parameters before doing task.
     }
 
     static Long lastRead(Channel channel) {
@@ -57,7 +57,7 @@ static Long now() {
         return System.currentTimeMillis();
     }
 
-    private void start() {
+    protected void start() {
         this.timeout = hashedWheelTimer.newTimeout(this, tick, TimeUnit.MILLISECONDS);
     }
 

dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/exchange/support/header/CloseTimerTask.java

@@ -34,6 +34,7 @@ public CloseTimerTask(
             ChannelProvider channelProvider, HashedWheelTimer hashedWheelTimer, Long tick, int closeTimeout) {
         super(channelProvider, hashedWheelTimer, tick);
         this.closeTimeout = closeTimeout;
+        start();
     }
 
     @Override

dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/exchange/support/header/HeartbeatTimerTask.java

@@ -36,6 +36,7 @@ public class HeartbeatTimerTask extends AbstractTimerTask {
             ChannelProvider channelProvider, HashedWheelTimer hashedWheelTimer, Long heartbeatTick, int heartbeat) {
         super(channelProvider, hashedWheelTimer, heartbeatTick);
         this.heartbeat = heartbeat;
+        start();
     }
 
     @Override

dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/exchange/support/header/ReconnectTimerTask.java

@@ -38,6 +38,7 @@ public ReconnectTimerTask(
             int idleTimeout) {
         super(channelProvider, hashedWheelTimer, heartbeatTimeoutTick);
         this.idleTimeout = idleTimeout;
+        start();
     }
 
     @Override

dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/transport/AbstractClient.java

@@ -50,7 +50,7 @@
 
 public abstract class AbstractClient extends AbstractEndpoint implements Client {
 
-    private final Lock connectLock = new ReentrantLock();
+    private Lock connectLock;
 
     private final boolean needReconnect;
 
@@ -64,6 +64,10 @@ public abstract class AbstractClient extends AbstractEndpoint implements Client
 
     public AbstractClient(URL url, ChannelHandler handler) throws RemotingException {
         super(url, handler);
+
+        // initialize connectLock before calling connect()
+        connectLock = new ReentrantLock();
+
         // set default needReconnect true when channel is not connected
         needReconnect = url.getParameter(Constants.SEND_RECONNECT_KEY, true);
 

dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/transport/MultiMessageHandler.java

@@ -37,7 +37,6 @@ public MultiMessageHandler(ChannelHandler handler) {
         super(handler);
     }
 
-    @SuppressWarnings("unchecked")
     @Override
     public void received(Channel channel, Object message) throws RemotingException {
         if (message instanceof MultiMessage) {

dubbo-remoting/dubbo-remoting-http3/src/main/java/org/apache/dubbo/remoting/transport/netty4/NettyHttp3Server.java

@@ -57,20 +57,23 @@ public class NettyHttp3Server extends AbstractServer {
     private EventLoopGroup bossGroup;
     private io.netty.channel.Channel channel;
 
-    private final Consumer<ChannelPipeline> pipelineConfigurator;
-    private final int serverShutdownTimeoutMills;
+    private Consumer<ChannelPipeline> pipelineConfigurator;
+    private int serverShutdownTimeoutMills;
 
-    @SuppressWarnings("unchecked")
     public NettyHttp3Server(URL url, ChannelHandler handler) throws RemotingException {
         super(url, ChannelHandlers.wrap(handler, url));
-        pipelineConfigurator = (Consumer<ChannelPipeline>) getUrl().getAttribute(PIPELINE_CONFIGURATOR_KEY);
-        Objects.requireNonNull(pipelineConfigurator, "pipelineConfigurator should be set");
-        serverShutdownTimeoutMills = ConfigurationUtils.getServerShutdownTimeout(getUrl().getOrDefaultModuleModel());
     }
 
     @Override
+    @SuppressWarnings("unchecked")
     protected void doOpen() throws Throwable {
         bootstrap = new Bootstrap();
+
+        // initialize pipelineConfigurator and serverShutdownTimeoutMills before potential usage to avoid NPE.
+        pipelineConfigurator = (Consumer<ChannelPipeline>) getUrl().getAttribute(PIPELINE_CONFIGURATOR_KEY);
+        Objects.requireNonNull(pipelineConfigurator, "pipelineConfigurator should be set");
+        serverShutdownTimeoutMills = ConfigurationUtils.getServerShutdownTimeout(getUrl().getOrDefaultModuleModel());
+
         bossGroup = NettyEventLoopFactory.eventLoopGroup(1, EVENT_LOOP_BOSS_POOL_NAME);
         NettyServerHandler nettyServerHandler = new NettyServerHandler(getUrl(), this);
         channels = nettyServerHandler.getChannels();

dubbo-remoting/dubbo-remoting-netty/src/main/java/org/apache/dubbo/remoting/transport/netty/NettyPortUnificationServer.java

@@ -32,7 +32,6 @@
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Map;
-import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 
@@ -56,7 +55,8 @@
  */
 public class NettyPortUnificationServer extends AbstractPortUnificationServer {
 
-    private Map<String, Channel> dubboChannels = new ConcurrentHashMap<>(); // <ip:port, channel>
+    // <ip:port, channel>
+    private Map<String, Channel> dubboChannels;
 
     private ServerBootstrap bootstrap;
 
@@ -95,6 +95,7 @@ protected void doOpen0() {
         bootstrap = new ServerBootstrap(channelFactory);
 
         final NettyHandler nettyHandler = new NettyHandler(getUrl(), this);
+        // set dubboChannels
         dubboChannels = nettyHandler.getChannels();
         // https://issues.jboss.org/browse/NETTY-365
         // https://issues.jboss.org/browse/NETTY-379

dubbo-remoting/dubbo-remoting-netty/src/main/java/org/apache/dubbo/remoting/transport/netty/NettyServer.java

@@ -54,7 +54,8 @@ public class NettyServer extends AbstractServer implements RemotingServer {
 
     private static final ErrorTypeAwareLogger logger = LoggerFactory.getErrorTypeAwareLogger(NettyServer.class);
 
-    private Map<String, Channel> channels; // <ip:port, channel>
+    // <ip:port, channel>
+    private Map<String, Channel> channels;
 
     private ServerBootstrap bootstrap;
 
@@ -75,6 +76,7 @@ protected void doOpen() throws Throwable {
         bootstrap = new ServerBootstrap(channelFactory);
 
         final NettyHandler nettyHandler = new NettyHandler(getUrl(), this);
+        // set channels
         channels = nettyHandler.getChannels();
         // https://issues.jboss.org/browse/NETTY-365
         // https://issues.jboss.org/browse/NETTY-379