[#3785] Correctly handle connection refused with native transport

Motivation:

Due a bug we not correctly handled connection refused errors and so failed the connect promise with the wrong exception.
Beside this we some times even triggered fireChannelActive() which is not correct.

Modifications:

- Add testcase
- correctly detect connect errors

Result:

Correct and consistent handling.

testsuite/src/main/java/io/netty/testsuite/transport/AbstractTestsuiteTest.java

@@ -57,7 +57,7 @@ public abstract class AbstractTestsuiteTest<T extends AbstractBootstrap<?, ?>> {
                         "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) {

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<Error> 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

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 {

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.

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<TestsuitePermutation.BootstrapFactory<Bootstrap>> newFactories() {
+        return EpollSocketTestPermutation.INSTANCE.clientSocket();
+    }
+}