andreacavalli/netty5
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Make NIO and EPOLL transport connect errors more consistent with the JDK

Browse Source 
Motivation:

The NIO transport used an IllegalStateException if a user tried to issue another connect(...) while the connect was still in process. For this case the JDK specified a ConnectPendingException which we should use. The same issues exists in the EPOLL transport. Beside this the EPOLL transport also does not throw the right exceptions for ENETUNREACH and EISCONN errno codes.

Modifications:

- Replace IllegalStateException with ConnectPendingException in NIO and EPOLL transport
- throw correct exceptions for ENETUNREACH and EISCONN in EPOLL transport
- Add test case

Result:

More correct error handling for connect attempts when using NIO and EPOLL transport
This commit is contained in:
Norman Maurer 2016-08-18 16:58:17 +02:00
parent 3a6157e2aa
commit d3cb95ef00
8 changed files with 165 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

74
testsuite/src/main/java/io/netty/testsuite/transport/socket/SocketMultipleConnectTest.java Normal file
View File

@ -0,0 +1,74 @@
/*
* Copyright 2016 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.testsuite.transport.socket;
import io.netty.bootstrap.Bootstrap;
import io.netty.bootstrap.ServerBootstrap;
import io.netty.channel.Channel;
import io.netty.channel.ChannelFuture;
import io.netty.channel.ChannelInboundHandlerAdapter;
import io.netty.channel.nio.NioEventLoopGroup;
import io.netty.testsuite.transport.TestsuitePermutation;
import org.junit.Test;
import java.nio.channels.AlreadyConnectedException;
import java.util.ArrayList;
import java.util.List;
import static org.junit.Assert.assertTrue;
public class SocketMultipleConnectTest extends AbstractSocketTest {
@Test(timeout = 30000)
public void testMultipleConnect() throws Throwable {
run();
}
public void testMultipleConnect(ServerBootstrap sb, Bootstrap cb) throws Exception {
Channel sc = null;
Channel cc = null;
try {
sb.childHandler(new ChannelInboundHandlerAdapter());
sc = sb.bind(0).syncUninterruptibly().channel();
cb.handler(new ChannelInboundHandlerAdapter());
cc = cb.register().syncUninterruptibly().channel();
cc.connect(sc.localAddress()).syncUninterruptibly();
ChannelFuture connectFuture2 = cc.connect(sc.localAddress()).await();
assertTrue(connectFuture2.cause() instanceof AlreadyConnectedException);
} finally {
if (cc != null) {
cc.close();
}
if (sc != null) {
sc.close();
}
}
}
@Override
protected List<TestsuitePermutation.BootstrapComboFactory<ServerBootstrap, Bootstrap>> newFactories() {
List<TestsuitePermutation.BootstrapComboFactory<ServerBootstrap, Bootstrap>> factories
= new ArrayList<TestsuitePermutation.BootstrapComboFactory<ServerBootstrap, Bootstrap>>();
for (TestsuitePermutation.BootstrapComboFactory<ServerBootstrap, Bootstrap> comboFactory
: SocketTestPermutation.INSTANCE.socket()) {
if (comboFactory.newClientInstance().config().group() instanceof NioEventLoopGroup) {
factories.add(comboFactory);
}
}
return factories;
}
}

15
transport-native-epoll/src/main/c/netty_unix_errors.c
View File

@ -104,6 +104,18 @@ static jint netty_unix_errors_errorECONNREFUSED(JNIEnv* env, jclass clazz) {
return ECONNREFUSED;
}
static jint netty_unix_errors_errorEISCONN(JNIEnv* env, jclass clazz) {
return EISCONN;
}
static jint netty_unix_errors_errorEALREADY(JNIEnv* env, jclass clazz) {
return EALREADY;
}
static jint netty_unix_errors_errorENETUNREACH(JNIEnv* env, jclass clazz) {
return ENETUNREACH;
}
static jstring netty_unix_errors_strError(JNIEnv* env, jclass clazz, jint error) {
return (*env)->NewStringUTF(env, strerror(error));
}
@ -119,6 +131,9 @@ static const JNINativeMethod statically_referenced_fixed_method_table[] = {
{ "errnoEWOULDBLOCK", "()I", (void *) netty_unix_errors_errnoEWOULDBLOCK },
{ "errnoEINPROGRESS", "()I", (void *) netty_unix_errors_errnoEINPROGRESS },
{ "errorECONNREFUSED", "()I", (void *) netty_unix_errors_errorECONNREFUSED },
{ "errorEISCONN", "()I", (void *) netty_unix_errors_errorEISCONN },
{ "errorEALREADY", "()I", (void *) netty_unix_errors_errorEALREADY },
{ "errorENETUNREACH", "()I", (void *) netty_unix_errors_errorENETUNREACH },
{ "strError", "(I)Ljava/lang/String;", (void *) netty_unix_errors_strError }
};
static const jint statically_referenced_fixed_method_table_size = sizeof(statically_referenced_fixed_method_table) / sizeof(statically_referenced_fixed_method_table[0]);

3
transport-native-epoll/src/main/java/io/netty/channel/epoll/AbstractEpollStreamChannel.java
View File

@ -43,6 +43,7 @@ import java.io.IOException;
import java.net.SocketAddress;
import java.nio.ByteBuffer;
import java.nio.channels.ClosedChannelException;
import java.nio.channels.ConnectionPendingException;
import java.util.Queue;
import java.util.concurrent.Executor;
import java.util.concurrent.ScheduledFuture;
@ -782,7 +783,7 @@ public abstract class AbstractEpollStreamChannel extends AbstractEpollChannel im
try {
if (connectPromise != null) {
throw new IllegalStateException("connection attempt already made");
throw new ConnectionPendingException();
}
boolean wasActive = isActive();

8
transport-native-epoll/src/main/java/io/netty/channel/epoll/EpollSocketChannel.java
View File

@ -29,6 +29,7 @@ import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.net.SocketAddress;
import java.net.UnknownHostException;
import java.nio.channels.AlreadyConnectedException;
import java.util.Collection;
import java.util.Collections;
import java.util.Map;
@ -182,6 +183,13 @@ public final class EpollSocketChannel extends AbstractEpollStreamChannel impleme
InetSocketAddress remoteAddr = (InetSocketAddress) remoteAddress;
checkResolvable(remoteAddr);
if (remote != null) {
// Check if already connected before trying to connect. This is needed as connect(...) will not return -1
// and set errno to EISCONN if a previous connect(...) attempt was setting errno to EINPROGRESS and finished
// later.
throw new AlreadyConnectedException();
}
boolean connected = super.doConnect(remoteAddress, localAddress);
if (connected) {
remote = computeRemoteAddr(remoteAddr, fd().remoteAddress());

27
transport-native-epoll/src/main/java/io/netty/channel/unix/Errors.java
View File

@ -19,18 +19,13 @@ import io.netty.util.internal.EmptyArrays;
import java.io.IOException;
import java.net.ConnectException;
import java.net.NoRouteToHostException;
import java.nio.channels.AlreadyConnectedException;
import java.nio.channels.ClosedChannelException;
import java.nio.channels.ConnectionPendingException;
import java.nio.channels.NotYetConnectedException;
import static io.netty.channel.unix.ErrorsStaticallyReferencedJniMethods.errnoEAGAIN;
import static io.netty.channel.unix.ErrorsStaticallyReferencedJniMethods.errnoEBADF;
import static io.netty.channel.unix.ErrorsStaticallyReferencedJniMethods.errnoECONNRESET;
import static io.netty.channel.unix.ErrorsStaticallyReferencedJniMethods.errnoEINPROGRESS;
import static io.netty.channel.unix.ErrorsStaticallyReferencedJniMethods.errnoENOTCONN;
import static io.netty.channel.unix.ErrorsStaticallyReferencedJniMethods.errnoEPIPE;
import static io.netty.channel.unix.ErrorsStaticallyReferencedJniMethods.errnoEWOULDBLOCK;
import static io.netty.channel.unix.ErrorsStaticallyReferencedJniMethods.errorECONNREFUSED;
import static io.netty.channel.unix.ErrorsStaticallyReferencedJniMethods.strError;
import static io.netty.channel.unix.ErrorsStaticallyReferencedJniMethods.*;
/**
* <strong>Internal usage only!</strong>
@ -46,6 +41,9 @@ public final class Errors {
public static final int ERRNO_EWOULDBLOCK_NEGATIVE = -errnoEWOULDBLOCK();
public static final int ERRNO_EINPROGRESS_NEGATIVE = -errnoEINPROGRESS();
public static final int ERROR_ECONNREFUSED_NEGATIVE = -errorECONNREFUSED();
public static final int ERROR_EISCONN_NEGATIVE = -errorEISCONN();
public static final int ERROR_EALREADY_NEGATIVE = -errorEALREADY();
public static final int ERROR_ENETUNREACH_NEGATIVE = -errorENETUNREACH();
/**
* Holds the mappings for errno codes to String messages.
@ -93,10 +91,19 @@ public final class Errors {
}
static void throwConnectException(String method, NativeConnectException refusedCause, int err)
throws ConnectException {
throws IOException {
if (err == refusedCause.expectedErr()) {
throw refusedCause;
}
if (err == ERROR_EALREADY_NEGATIVE) {
throw new ConnectionPendingException();
}
if (err == ERROR_ENETUNREACH_NEGATIVE) {
throw new NoRouteToHostException();
}
if (err == ERROR_EISCONN_NEGATIVE) {
throw new AlreadyConnectedException();
}
throw new ConnectException(method + "() failed: " + ERRORS[-err]);
}

3
transport-native-epoll/src/main/java/io/netty/channel/unix/ErrorsStaticallyReferencedJniMethods.java
View File

@ -38,5 +38,8 @@ final class ErrorsStaticallyReferencedJniMethods {
static native int errnoEWOULDBLOCK();
static native int errnoEINPROGRESS();
static native int errorECONNREFUSED();
static native int errorEISCONN();
static native int errorEALREADY();
static native int errorENETUNREACH();
static native String strError(int err);
}

43
transport-native-epoll/src/test/java/io/netty/channel/epoll/EpollSocketMultipleConnectTest.java Normal file
View File

@ -0,0 +1,43 @@
/*
* Copyright 2016 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.bootstrap.ServerBootstrap;
import io.netty.channel.EventLoopGroup;
import io.netty.channel.nio.NioEventLoopGroup;
import io.netty.testsuite.transport.TestsuitePermutation;
import io.netty.testsuite.transport.socket.SocketMultipleConnectTest;
import java.util.ArrayList;
import java.util.List;
public class EpollSocketMultipleConnectTest extends SocketMultipleConnectTest {
@Override
protected List<TestsuitePermutation.BootstrapComboFactory<ServerBootstrap, Bootstrap>> newFactories() {
List<TestsuitePermutation.BootstrapComboFactory<ServerBootstrap, Bootstrap>> factories
= new ArrayList<TestsuitePermutation.BootstrapComboFactory<ServerBootstrap, Bootstrap>>();
for (TestsuitePermutation.BootstrapComboFactory<ServerBootstrap, Bootstrap> comboFactory
: EpollSocketTestPermutation.INSTANCE.socket()) {
EventLoopGroup group = comboFactory.newClientInstance().config().group();
if (group instanceof NioEventLoopGroup || group instanceof EpollEventLoopGroup) {
factories.add(comboFactory);
}
}
return factories;
}
}

4
transport/src/main/java/io/netty/channel/nio/AbstractNioChannel.java
View File

@ -37,6 +37,7 @@ import java.io.IOException;
import java.net.SocketAddress;
import java.nio.channels.CancelledKeyException;
import java.nio.channels.ClosedChannelException;
import java.nio.channels.ConnectionPendingException;
import java.nio.channels.SelectableChannel;
import java.nio.channels.SelectionKey;
import java.util.concurrent.ScheduledFuture;
@ -245,7 +246,8 @@ public abstract class AbstractNioChannel extends AbstractChannel {
try {
if (connectPromise != null) {
throw new IllegalStateException("connection attempt already made");
// Already a connect in process.
throw new ConnectionPendingException();
}
boolean wasActive = isActive();