diff --git a/testsuite/src/main/java/io/netty/testsuite/transport/AbstractTestsuiteTest.java b/testsuite/src/main/java/io/netty/testsuite/transport/AbstractTestsuiteTest.java index 5ec58109df..a43d7bf019 100644 --- a/testsuite/src/main/java/io/netty/testsuite/transport/AbstractTestsuiteTest.java +++ b/testsuite/src/main/java/io/netty/testsuite/transport/AbstractTestsuiteTest.java @@ -57,7 +57,7 @@ public abstract class AbstractTestsuiteTest> { "Running: %s %d of %d with %s", testName.getMethodName(), ++ i, combos.size(), StringUtil.simpleClassName(allocator))); try { - Method m = getClass().getDeclaredMethod( + Method m = getClass().getMethod( TestUtils.testMethodName(testName), clazz); m.invoke(this, cb); } catch (InvocationTargetException ex) { diff --git a/testsuite/src/main/java/io/netty/testsuite/transport/socket/SocketConnectionAttemptTest.java b/testsuite/src/main/java/io/netty/testsuite/transport/socket/SocketConnectionAttemptTest.java index b8a1d69e87..3c716c8ada 100644 --- a/testsuite/src/main/java/io/netty/testsuite/transport/socket/SocketConnectionAttemptTest.java +++ b/testsuite/src/main/java/io/netty/testsuite/transport/socket/SocketConnectionAttemptTest.java @@ -16,10 +16,16 @@ package io.netty.testsuite.transport.socket; import io.netty.bootstrap.Bootstrap; +import io.netty.channel.Channel; import io.netty.channel.ChannelFuture; +import io.netty.channel.ChannelHandler; import io.netty.channel.ChannelHandlerContext; import io.netty.channel.ChannelInboundHandlerAdapter; import io.netty.channel.ChannelOption; +import io.netty.testsuite.util.TestUtils; +import io.netty.util.NetUtil; +import io.netty.util.concurrent.GlobalEventExecutor; +import io.netty.util.concurrent.Promise; import io.netty.util.internal.SystemPropertyUtil; import io.netty.util.internal.logging.InternalLogger; import io.netty.util.internal.logging.InternalLoggerFactory; @@ -60,6 +66,41 @@ public class SocketConnectionAttemptTest extends AbstractClientSocketTest { } } + @Test(timeout = 30000) + public void testConnectRefused() throws Throwable { + run(); + } + + public void testConnectRefused(Bootstrap cb) throws Throwable { + testConnectRefused0(cb, false); + } + + @Test(timeout = 30000) + public void testConnectRefusedHalfClosure() throws Throwable { + run(); + } + + public void testConnectRefusedHalfClosure(Bootstrap cb) throws Throwable { + testConnectRefused0(cb, true); + } + + private static void testConnectRefused0(Bootstrap cb, boolean halfClosure) throws Throwable { + final Promise errorPromise = GlobalEventExecutor.INSTANCE.newPromise(); + ChannelHandler handler = new ChannelInboundHandlerAdapter() { + @Override + public void channelActive(ChannelHandlerContext ctx) throws Exception { + Channel channel = ctx.channel(); + errorPromise.setFailure(new AssertionError("should have never been called")); + } + }; + + cb.handler(handler); + cb.option(ChannelOption.ALLOW_HALF_CLOSURE, halfClosure); + ChannelFuture future = cb.connect(NetUtil.LOCALHOST, TestUtils.getFreePort()).awaitUninterruptibly(); + assertTrue(future.cause() instanceof ConnectException); + assertNull(errorPromise.cause()); + } + @Test public void testConnectCancellation() throws Throwable { // Check if the test can be executed or should be skipped because of no network/internet connection diff --git a/transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollEventLoop.java b/transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollEventLoop.java index 133ed067f4..22b414f3ed 100644 --- a/transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollEventLoop.java +++ b/transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollEventLoop.java @@ -309,9 +309,21 @@ final class EpollEventLoop extends SingleThreadEventLoop { AbstractEpollChannel ch = channels.get(fd); if (ch != null) { + // Don't change the ordering of processing EPOLLOUT | EPOLLRDHUP / EPOLLIN if you're not 100% + // sure about it! + // Re-ordering can easily introduce bugs and bad side-effects, as we found out painfully in the + // past. AbstractEpollUnsafe unsafe = (AbstractEpollUnsafe) ch.unsafe(); - // First check if EPOLLRDHUP was set, this will notify us for connection-reset in which case + // First check for EPOLLOUT as we may need to fail the connect ChannelPromise before try + // to read from the file descriptor. + // See https://github.com/netty/netty/issues/3785 + if ((ev & Native.EPOLLOUT) != 0 && ch.isOpen()) { + // Force flush of data as the epoll is writable again + unsafe.epollOutReady(); + } + + // Check if EPOLLRDHUP was set, this will notify us for connection-reset in which case // we may close the channel directly or try to read more data depending on the state of the // Channel and als depending on the AbstractEpollChannel subtype. if ((ev & Native.EPOLLRDHUP) != 0) { @@ -321,10 +333,6 @@ final class EpollEventLoop extends SingleThreadEventLoop { // The Channel is still open and there is something to read. Do it now. unsafe.epollInReady(); } - if ((ev & Native.EPOLLOUT) != 0 && ch.isOpen()) { - // Force flush of data as the epoll is writable again - unsafe.epollOutReady(); - } } else { // We received an event for an fd which we not use anymore. Remove it from the epoll_event set. try { diff --git a/transport-native-epoll/src/main/java/io/netty/channel/epoll/Native.java b/transport-native-epoll/src/main/java/io/netty/channel/epoll/Native.java index 91a9f1a792..f1922221f1 100644 --- a/transport-native-epoll/src/main/java/io/netty/channel/epoll/Native.java +++ b/transport-native-epoll/src/main/java/io/netty/channel/epoll/Native.java @@ -25,6 +25,7 @@ import io.netty.util.internal.PlatformDependent; import io.netty.util.internal.SystemPropertyUtil; import java.io.IOException; +import java.net.ConnectException; import java.net.Inet6Address; import java.net.InetAddress; import java.net.InetSocketAddress; @@ -471,7 +472,7 @@ public final class Native { // connect not complete yet need to wait for EPOLLOUT event return false; } - throw newIOException("connect", res); + throw newConnectException("connect", res); } return true; } @@ -486,13 +487,17 @@ public final class Native { // connect still in progress return false; } - throw newIOException("finishConnect", res); + throw newConnectException("finishConnect", res); } return true; } private static native int finishConnect0(int fd); + private static ConnectException newConnectException(String method, int err) { + return new ConnectException(method + "() failed: " + ERRORS[-err]); + } + public static InetSocketAddress remoteAddress(int fd) { byte[] addr = remoteAddress0(fd); // addr may be null if getpeername failed. diff --git a/transport-native-epoll/src/test/java/io/netty/channel/epoll/EpollSocketConnectionAttemptTest.java b/transport-native-epoll/src/test/java/io/netty/channel/epoll/EpollSocketConnectionAttemptTest.java new file mode 100644 index 0000000000..a4fddfeb38 --- /dev/null +++ b/transport-native-epoll/src/test/java/io/netty/channel/epoll/EpollSocketConnectionAttemptTest.java @@ -0,0 +1,29 @@ +/* + * Copyright 2015 The Netty Project + * + * The Netty Project 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 io.netty.channel.epoll; + +import io.netty.bootstrap.Bootstrap; +import io.netty.testsuite.transport.TestsuitePermutation; +import io.netty.testsuite.transport.socket.SocketConnectionAttemptTest; + +import java.util.List; + +public class EpollSocketConnectionAttemptTest extends SocketConnectionAttemptTest { + @Override + protected List> newFactories() { + return EpollSocketTestPermutation.INSTANCE.clientSocket(); + } +}