You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@dubbo.apache.org by ea...@apache.org on 2025/08/01 07:04:37 UTC
(dubbo) branch 3.3 updated: Fix uninitialized non-static final fields potential NPE issue in dubbo-remoting module (#15602)
This is an automated email from the ASF dual-hosted git repository.
earthchen pushed a commit to branch 3.3
in repository https://gitbox.apache.org/repos/asf/dubbo.git
The following commit(s) were added to refs/heads/3.3 by this push:
new af2625c6de Fix uninitialized non-static final fields potential NPE issue in dubbo-remoting module (#15602)
af2625c6de is described below
commit af2625c6deb13b9b5f4193ce6c7b78115062ad73
Author: zrlw <zr...@sina.com>
AuthorDate: Fri Aug 1 15:04:31 2025 +0800
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 <ea...@gmail.com>
---
.../api/pu/AbstractPortUnificationServer.java | 8 ++++++--
.../exchange/support/header/AbstractTimerTask.java | 4 ++--
.../exchange/support/header/CloseTimerTask.java | 1 +
.../exchange/support/header/HeartbeatTimerTask.java | 1 +
.../exchange/support/header/ReconnectTimerTask.java | 1 +
.../dubbo/remoting/transport/AbstractClient.java | 6 +++++-
.../dubbo/remoting/transport/MultiMessageHandler.java | 1 -
.../remoting/transport/netty4/NettyHttp3Server.java | 15 +++++++++------
.../transport/netty/NettyPortUnificationServer.java | 5 +++--
.../dubbo/remoting/transport/netty/NettyServer.java | 4 +++-
.../transport/netty4/NettyPortUnificationServer.java | 19 +++++++++++--------
.../dubbo/remoting/transport/netty4/NettyServer.java | 9 +++++----
12 files changed, 47 insertions(+), 27 deletions(-)
diff --git a/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/api/pu/AbstractPortUnificationServer.java b/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/api/pu/AbstractPortUnificationServer.java
index d7d4e0f461..c3a1513328 100644
--- a/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/api/pu/AbstractPortUnificationServer.java
+++ b/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 abstract class AbstractPortUnificationServer extends AbstractServer {
@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()
diff --git a/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/exchange/support/header/AbstractTimerTask.java b/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/exchange/support/header/AbstractTimerTask.java
index 1512450d7f..53dcbdcb05 100644
--- a/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/exchange/support/header/AbstractTimerTask.java
+++ b/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 @@ public abstract class AbstractTimerTask implements TimerTask {
return System.currentTimeMillis();
}
- private void start() {
+ protected void start() {
this.timeout = hashedWheelTimer.newTimeout(this, tick, TimeUnit.MILLISECONDS);
}
diff --git a/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/exchange/support/header/CloseTimerTask.java b/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/exchange/support/header/CloseTimerTask.java
index b15b98eb9e..54dc5f126a 100644
--- a/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/exchange/support/header/CloseTimerTask.java
+++ b/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/exchange/support/header/CloseTimerTask.java
@@ -34,6 +34,7 @@ public class CloseTimerTask extends AbstractTimerTask {
ChannelProvider channelProvider, HashedWheelTimer hashedWheelTimer, Long tick, int closeTimeout) {
super(channelProvider, hashedWheelTimer, tick);
this.closeTimeout = closeTimeout;
+ start();
}
@Override
diff --git a/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/exchange/support/header/HeartbeatTimerTask.java b/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/exchange/support/header/HeartbeatTimerTask.java
index be2717adfe..d1fda4947c 100644
--- a/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/exchange/support/header/HeartbeatTimerTask.java
+++ b/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
diff --git a/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/exchange/support/header/ReconnectTimerTask.java b/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/exchange/support/header/ReconnectTimerTask.java
index 84f42fab96..0ce40b593c 100644
--- a/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/exchange/support/header/ReconnectTimerTask.java
+++ b/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/exchange/support/header/ReconnectTimerTask.java
@@ -38,6 +38,7 @@ public class ReconnectTimerTask extends AbstractTimerTask {
int idleTimeout) {
super(channelProvider, hashedWheelTimer, heartbeatTimeoutTick);
this.idleTimeout = idleTimeout;
+ start();
}
@Override
diff --git a/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/transport/AbstractClient.java b/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/transport/AbstractClient.java
index d0f0503e54..b1039b3a37 100644
--- a/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/transport/AbstractClient.java
+++ b/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/transport/AbstractClient.java
@@ -50,7 +50,7 @@ import static org.apache.dubbo.remoting.utils.UrlUtils.getIdleTimeout;
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);
diff --git a/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/transport/MultiMessageHandler.java b/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/transport/MultiMessageHandler.java
index 9cb55fb514..fb0073d912 100644
--- a/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/transport/MultiMessageHandler.java
+++ b/dubbo-remoting/dubbo-remoting-api/src/main/java/org/apache/dubbo/remoting/transport/MultiMessageHandler.java
@@ -37,7 +37,6 @@ public class MultiMessageHandler extends AbstractChannelHandlerDelegate {
super(handler);
}
- @SuppressWarnings("unchecked")
@Override
public void received(Channel channel, Object message) throws RemotingException {
if (message instanceof MultiMessage) {
diff --git a/dubbo-remoting/dubbo-remoting-http3/src/main/java/org/apache/dubbo/remoting/transport/netty4/NettyHttp3Server.java b/dubbo-remoting/dubbo-remoting-http3/src/main/java/org/apache/dubbo/remoting/transport/netty4/NettyHttp3Server.java
index c3fcbe9f18..38a59c5103 100644
--- a/dubbo-remoting/dubbo-remoting-http3/src/main/java/org/apache/dubbo/remoting/transport/netty4/NettyHttp3Server.java
+++ b/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();
diff --git a/dubbo-remoting/dubbo-remoting-netty/src/main/java/org/apache/dubbo/remoting/transport/netty/NettyPortUnificationServer.java b/dubbo-remoting/dubbo-remoting-netty/src/main/java/org/apache/dubbo/remoting/transport/netty/NettyPortUnificationServer.java
index f35e743aab..f616dcb932 100644
--- a/dubbo-remoting/dubbo-remoting-netty/src/main/java/org/apache/dubbo/remoting/transport/netty/NettyPortUnificationServer.java
+++ b/dubbo-remoting/dubbo-remoting-netty/src/main/java/org/apache/dubbo/remoting/transport/netty/NettyPortUnificationServer.java
@@ -32,7 +32,6 @@ import java.net.InetSocketAddress;
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 @@ import static org.apache.dubbo.remoting.Constants.EVENT_LOOP_WORKER_POOL_NAME;
*/
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 @@ public class NettyPortUnificationServer extends AbstractPortUnificationServer {
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
diff --git a/dubbo-remoting/dubbo-remoting-netty/src/main/java/org/apache/dubbo/remoting/transport/netty/NettyServer.java b/dubbo-remoting/dubbo-remoting-netty/src/main/java/org/apache/dubbo/remoting/transport/netty/NettyServer.java
index 3d88453e61..73f731bd1e 100644
--- a/dubbo-remoting/dubbo-remoting-netty/src/main/java/org/apache/dubbo/remoting/transport/netty/NettyServer.java
+++ b/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 @@ public class NettyServer extends AbstractServer implements RemotingServer {
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
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 16a17c957f..8bbdfb188f 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
@@ -57,7 +57,7 @@ import static org.apache.dubbo.remoting.Constants.EVENT_LOOP_WORKER_POOL_NAME;
*/
public class NettyPortUnificationServer extends AbstractPortUnificationServer {
- private final int serverShutdownTimeoutMills;
+ private int serverShutdownTimeoutMills;
/**
* netty server bootstrap.
*/
@@ -69,16 +69,10 @@ public class NettyPortUnificationServer extends AbstractPortUnificationServer {
private EventLoopGroup bossGroup;
private EventLoopGroup workerGroup;
- private final Map<String, Channel> dubboChannels = new ConcurrentHashMap<>();
+ private Map<String, Channel> dubboChannels;
public NettyPortUnificationServer(URL url, ChannelHandler handler) throws RemotingException {
super(url, ChannelHandlers.wrap(handler, url));
-
- // you can customize name and type of client thread pool by THREAD_NAME_KEY and THREADPOOL_KEY in
- // CommonConstants.
- // the handler will be wrapped: MultiMessageHandler->HeartbeatHandler->handler
- // read config before destroy
- serverShutdownTimeoutMills = ConfigurationUtils.getServerShutdownTimeout(getUrl().getOrDefaultModuleModel());
}
@Override
@@ -103,6 +97,15 @@ public class NettyPortUnificationServer extends AbstractPortUnificationServer {
public void doOpen0() {
bootstrap = new ServerBootstrap();
+ // initialize dubboChannels and serverShutdownTimeoutMills before potential usage to avoid NPE.
+ dubboChannels = new ConcurrentHashMap<>();
+
+ // you can customize name and type of client thread pool by THREAD_NAME_KEY and THREADPOOL_KEY in
+ // CommonConstants.
+ // the handler will be wrapped: MultiMessageHandler->HeartbeatHandler->handler
+ // read config before destroy
+ serverShutdownTimeoutMills = ConfigurationUtils.getServerShutdownTimeout(getUrl().getOrDefaultModuleModel());
+
bossGroup = NettyEventLoopFactory.eventLoopGroup(1, EVENT_LOOP_BOSS_POOL_NAME);
workerGroup = NettyEventLoopFactory.eventLoopGroup(
getUrl().getPositiveParameter(IO_THREADS_KEY, Constants.DEFAULT_IO_THREADS),
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 504931c90f..b5a7b29c65 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
@@ -78,16 +78,13 @@ public class NettyServer extends AbstractServer {
private EventLoopGroup bossGroup;
private EventLoopGroup workerGroup;
- private final int serverShutdownTimeoutMills;
+ private int serverShutdownTimeoutMills;
public NettyServer(URL url, ChannelHandler handler) throws RemotingException {
// you can customize name and type of client thread pool by THREAD_NAME_KEY and THREAD_POOL_KEY in
// CommonConstants.
// the handler will be wrapped: MultiMessageHandler->HeartbeatHandler->handler
super(url, ChannelHandlers.wrap(handler, url));
-
- // read config before destroy
- serverShutdownTimeoutMills = ConfigurationUtils.getServerShutdownTimeout(getUrl().getOrDefaultModuleModel());
}
/**
@@ -99,6 +96,10 @@ public class NettyServer extends AbstractServer {
protected void doOpen() throws Throwable {
bootstrap = new ServerBootstrap();
+ // initialize serverShutdownTimeoutMills before potential usage to avoid NPE.
+ // read config before destroy
+ serverShutdownTimeoutMills = ConfigurationUtils.getServerShutdownTimeout(getUrl().getOrDefaultModuleModel());
+
bossGroup = createBossGroup();
workerGroup = createWorkerGroup();