From 61b5792340197b38b7fb4be7168835c429bd5315 Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Tue, 24 Nov 2015 04:54:39 +0100 Subject: [PATCH] Fix race-condition when closing a NioSocketChannel or EpollSocketChannel Motivation: Fix a race-condition when closing NioSocketChannel or EpollSocketChannel while try to detect if a close executor should be used and the underlying socket was already closed. This could lead to an exception that then leave the channel / in an invalid state and so could lead to side-effects like heavy CPU usage. Modifications: Catch possible socket exception while try to get the SO_LINGER options from the underlying socket. Result: No more race-condition when closing the channel is possible with bad side-effects. --- .../io/netty/channel/epoll/EpollSocketChannel.java | 14 ++++++++++---- .../netty/channel/socket/nio/NioSocketChannel.java | 11 +++++++++-- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollSocketChannel.java b/transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollSocketChannel.java index 72a2b6ae1b..ec14d4aec7 100644 --- a/transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollSocketChannel.java +++ b/transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollSocketChannel.java @@ -219,10 +219,16 @@ public final class EpollSocketChannel extends AbstractEpollStreamChannel impleme private final class EpollSocketChannelUnsafe extends EpollStreamUnsafe { @Override protected Executor closeExecutor() { - // Check isOpen() first as otherwise it will throw a RuntimeException - // when call getSoLinger() as the fd is not valid anymore. - if (isOpen() && config().getSoLinger() > 0) { - return GlobalEventExecutor.INSTANCE; + try { + // Check isOpen() first as otherwise it will throw a RuntimeException + // when call getSoLinger() as the fd is not valid anymore. + if (isOpen() && config().getSoLinger() > 0) { + return GlobalEventExecutor.INSTANCE; + } + } catch (Throwable ignore) { + // Ignore the error as the underlying channel may be closed in the meantime and so + // getSoLinger() may produce an exception. In this case we just return null. + // See https://github.com/netty/netty/issues/4449 } return null; } diff --git a/transport/src/main/java/io/netty/channel/socket/nio/NioSocketChannel.java b/transport/src/main/java/io/netty/channel/socket/nio/NioSocketChannel.java index 5d7aa121ba..af2520d96e 100644 --- a/transport/src/main/java/io/netty/channel/socket/nio/NioSocketChannel.java +++ b/transport/src/main/java/io/netty/channel/socket/nio/NioSocketChannel.java @@ -35,6 +35,7 @@ import java.io.IOException; import java.net.InetSocketAddress; import java.net.Socket; import java.net.SocketAddress; +import java.net.SocketException; import java.nio.ByteBuffer; import java.nio.channels.SelectionKey; import java.nio.channels.SocketChannel; @@ -333,8 +334,14 @@ public class NioSocketChannel extends AbstractNioByteChannel implements io.netty private final class NioSocketChannelUnsafe extends NioByteUnsafe { @Override protected Executor closeExecutor() { - if (javaChannel().isOpen() && config().getSoLinger() > 0) { - return GlobalEventExecutor.INSTANCE; + try { + if (javaChannel().isOpen() && config().getSoLinger() > 0) { + return GlobalEventExecutor.INSTANCE; + } + } catch (Throwable ignore) { + // Ignore the error as the underlying channel may be closed in the meantime and so + // getSoLinger() may produce an exception. In this case we just return null. + // See https://github.com/netty/netty/issues/4449 } return null; }