From 14e1d0f9f76f0d1eb0bff366e16f42752d2699f5 Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Sun, 19 Apr 2015 21:07:19 +0200 Subject: [PATCH] [#3662] Fail the connect future on close Motivation: Because of a bug we missed to fail the connect future when doClose() is called. This can lead to a future which is never notified and so may lead to deadlocks in user-programs. Modifications: Correctly fail the connect future when doClose() is called and the connection was not established yet. Result: Connect future is always notified. --- .../epoll/AbstractEpollStreamChannel.java | 41 +++++++++++++++---- .../netty/channel/nio/AbstractNioChannel.java | 24 +++++++++++ .../channel/socket/nio/NioSocketChannel.java | 1 + 3 files changed, 58 insertions(+), 8 deletions(-) diff --git a/transport-native-epoll/src/main/java/io/netty/channel/epoll/AbstractEpollStreamChannel.java b/transport-native-epoll/src/main/java/io/netty/channel/epoll/AbstractEpollStreamChannel.java index 4988a8e422..9d63974dd8 100644 --- a/transport-native-epoll/src/main/java/io/netty/channel/epoll/AbstractEpollStreamChannel.java +++ b/transport-native-epoll/src/main/java/io/netty/channel/epoll/AbstractEpollStreamChannel.java @@ -31,12 +31,14 @@ import io.netty.channel.DefaultFileRegion; import io.netty.channel.RecvByteBufAllocator; import io.netty.channel.socket.ChannelInputShutdownEvent; import io.netty.channel.unix.FileDescriptor; +import io.netty.util.internal.EmptyArrays; import io.netty.util.internal.PlatformDependent; import io.netty.util.internal.StringUtil; import java.io.IOException; import java.net.SocketAddress; import java.nio.ByteBuffer; +import java.nio.channels.ClosedChannelException; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; @@ -45,6 +47,19 @@ public abstract class AbstractEpollStreamChannel extends AbstractEpollChannel { private static final String EXPECTED_TYPES = " (expected: " + StringUtil.simpleClassName(ByteBuf.class) + ", " + StringUtil.simpleClassName(DefaultFileRegion.class) + ')'; + private static final ClosedChannelException CLOSED_CHANNEL_EXCEPTION = new ClosedChannelException(); + + static { + CLOSED_CHANNEL_EXCEPTION.setStackTrace(EmptyArrays.EMPTY_STACK_TRACE); + } + + /** + * The future of the current connection attempt. If not null, subsequent + * connection attempts will fail. + */ + private ChannelPromise connectPromise; + private ScheduledFuture connectTimeoutFuture; + private SocketAddress requestedRemoteAddress; private volatile boolean inputShutdown; private volatile boolean outputShutdown; @@ -388,14 +403,24 @@ public abstract class AbstractEpollStreamChannel extends AbstractEpollChannel { } } + @Override + protected void doClose() throws Exception { + ChannelPromise promise = connectPromise; + if (promise != null) { + // Use tryFailure() instead of setFailure() to avoid the race against cancel(). + promise.tryFailure(CLOSED_CHANNEL_EXCEPTION); + connectPromise = null; + } + + ScheduledFuture future = connectTimeoutFuture; + if (future != null) { + future.cancel(false); + connectTimeoutFuture = null; + } + super.doClose(); + } + class EpollStreamUnsafe extends AbstractEpollUnsafe { - /** - * The future of the current connection attempt. If not null, subsequent - * connection attempts will fail. - */ - private ChannelPromise connectPromise; - private ScheduledFuture connectTimeoutFuture; - private SocketAddress requestedRemoteAddress; private RecvByteBufAllocator.Handle allocHandle; @@ -454,7 +479,7 @@ public abstract class AbstractEpollStreamChannel extends AbstractEpollChannel { connectTimeoutFuture = eventLoop().schedule(new Runnable() { @Override public void run() { - ChannelPromise connectPromise = EpollStreamUnsafe.this.connectPromise; + ChannelPromise connectPromise = AbstractEpollStreamChannel.this.connectPromise; ConnectTimeoutException cause = new ConnectTimeoutException("connection timed out: " + remoteAddress); if (connectPromise != null && connectPromise.tryFailure(cause)) { diff --git a/transport/src/main/java/io/netty/channel/nio/AbstractNioChannel.java b/transport/src/main/java/io/netty/channel/nio/AbstractNioChannel.java index f75c4307b3..a7be69fa26 100644 --- a/transport/src/main/java/io/netty/channel/nio/AbstractNioChannel.java +++ b/transport/src/main/java/io/netty/channel/nio/AbstractNioChannel.java @@ -29,6 +29,7 @@ import io.netty.channel.ConnectTimeoutException; import io.netty.channel.EventLoop; import io.netty.util.ReferenceCountUtil; import io.netty.util.ReferenceCounted; +import io.netty.util.internal.EmptyArrays; import io.netty.util.internal.OneTimeTask; import io.netty.util.internal.logging.InternalLogger; import io.netty.util.internal.logging.InternalLoggerFactory; @@ -36,6 +37,7 @@ import io.netty.util.internal.logging.InternalLoggerFactory; import java.io.IOException; import java.net.SocketAddress; import java.nio.channels.CancelledKeyException; +import java.nio.channels.ClosedChannelException; import java.nio.channels.SelectableChannel; import java.nio.channels.SelectionKey; import java.util.concurrent.ScheduledFuture; @@ -49,6 +51,12 @@ public abstract class AbstractNioChannel extends AbstractChannel { private static final InternalLogger logger = InternalLoggerFactory.getInstance(AbstractNioChannel.class); + private static final ClosedChannelException CLOSED_CHANNEL_EXCEPTION = new ClosedChannelException(); + + static { + CLOSED_CHANNEL_EXCEPTION.setStackTrace(EmptyArrays.EMPTY_STACK_TRACE); + } + private final SelectableChannel ch; protected final int readInterestOp; volatile SelectionKey selectionKey; @@ -440,4 +448,20 @@ public abstract class AbstractNioChannel extends AbstractChannel { return buf; } + + @Override + protected void doClose() throws Exception { + ChannelPromise promise = connectPromise; + if (promise != null) { + // Use tryFailure() instead of setFailure() to avoid the race against cancel(). + promise.tryFailure(CLOSED_CHANNEL_EXCEPTION); + connectPromise = null; + } + + ScheduledFuture future = connectTimeoutFuture; + if (future != null) { + future.cancel(false); + connectTimeoutFuture = 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 718f10e4de..9c817bbbbb 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 @@ -232,6 +232,7 @@ public class NioSocketChannel extends AbstractNioByteChannel implements io.netty @Override protected void doClose() throws Exception { + super.doClose(); javaChannel().close(); }