From a391ffcb24f5b5396f24ab9eca18635c44d8acfc Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Tue, 19 Jun 2018 11:13:53 -0600 Subject: [PATCH 1/9] Ensure local and remote addresses aren't null Currently we set local and remote addresses on the creation time of a NioChannel. However, these may return null as they may not have been set yet. An example is the local address has not been set on a client channels as the connection process is not yet complete. This PR modifies the getters to set the local and remote address fields if they are currently null. --- .../main/java/org/elasticsearch/nio/NioChannel.java | 12 +++++++++--- .../elasticsearch/nio/NioServerSocketChannel.java | 3 +-- .../java/org/elasticsearch/nio/NioSocketChannel.java | 10 +++++++--- 3 files changed, 17 insertions(+), 8 deletions(-) diff --git a/libs/nio/src/main/java/org/elasticsearch/nio/NioChannel.java b/libs/nio/src/main/java/org/elasticsearch/nio/NioChannel.java index 2cc2bd260f0b2..cc34570dae61e 100644 --- a/libs/nio/src/main/java/org/elasticsearch/nio/NioChannel.java +++ b/libs/nio/src/main/java/org/elasticsearch/nio/NioChannel.java @@ -32,10 +32,11 @@ */ public abstract class NioChannel { - private final InetSocketAddress localAddress; + private final NetworkChannel socketChannel; + private volatile InetSocketAddress localAddress; - NioChannel(NetworkChannel socketChannel) throws IOException { - this.localAddress = (InetSocketAddress) socketChannel.getLocalAddress(); + NioChannel(NetworkChannel socketChannel) { + this.socketChannel = socketChannel; } public boolean isOpen() { @@ -43,6 +44,11 @@ public boolean isOpen() { } public InetSocketAddress getLocalAddress() { + if (localAddress == null) { + try { + localAddress = (InetSocketAddress) socketChannel.getLocalAddress(); + } catch (IOException e) {} + } return localAddress; } diff --git a/libs/nio/src/main/java/org/elasticsearch/nio/NioServerSocketChannel.java b/libs/nio/src/main/java/org/elasticsearch/nio/NioServerSocketChannel.java index 9f78c3b1b319d..6e8a42db60ba7 100644 --- a/libs/nio/src/main/java/org/elasticsearch/nio/NioServerSocketChannel.java +++ b/libs/nio/src/main/java/org/elasticsearch/nio/NioServerSocketChannel.java @@ -19,7 +19,6 @@ package org.elasticsearch.nio; -import java.io.IOException; import java.nio.channels.ServerSocketChannel; import java.util.concurrent.atomic.AtomicBoolean; @@ -29,7 +28,7 @@ public class NioServerSocketChannel extends NioChannel { private final AtomicBoolean contextSet = new AtomicBoolean(false); private ServerChannelContext context; - public NioServerSocketChannel(ServerSocketChannel socketChannel) throws IOException { + public NioServerSocketChannel(ServerSocketChannel socketChannel) { super(socketChannel); this.socketChannel = socketChannel; } diff --git a/libs/nio/src/main/java/org/elasticsearch/nio/NioSocketChannel.java b/libs/nio/src/main/java/org/elasticsearch/nio/NioSocketChannel.java index 32e934766913e..36e7dbc7aaa02 100644 --- a/libs/nio/src/main/java/org/elasticsearch/nio/NioSocketChannel.java +++ b/libs/nio/src/main/java/org/elasticsearch/nio/NioSocketChannel.java @@ -27,15 +27,14 @@ public class NioSocketChannel extends NioChannel { - private final InetSocketAddress remoteAddress; private final AtomicBoolean contextSet = new AtomicBoolean(false); private final SocketChannel socketChannel; + private volatile InetSocketAddress remoteAddress; private SocketChannelContext context; - public NioSocketChannel(SocketChannel socketChannel) throws IOException { + public NioSocketChannel(SocketChannel socketChannel) { super(socketChannel); this.socketChannel = socketChannel; - this.remoteAddress = (InetSocketAddress) socketChannel.getRemoteAddress(); } public void setContext(SocketChannelContext context) { @@ -57,6 +56,11 @@ public SocketChannelContext getContext() { } public InetSocketAddress getRemoteAddress() { + if (remoteAddress == null) { + try { + remoteAddress = (InetSocketAddress) socketChannel.getLocalAddress(); + } catch (IOException e) {} + } return remoteAddress; } From a9af9a0edcf2b0a9427a7bb75d684d317f50cf70 Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Tue, 19 Jun 2018 11:21:56 -0600 Subject: [PATCH 2/9] A few cleanups --- .../src/main/java/org/elasticsearch/nio/NioChannel.java | 7 +------ .../org/elasticsearch/nio/NioServerSocketChannel.java | 1 - .../main/java/org/elasticsearch/nio/NioSocketChannel.java | 1 - .../java/org/elasticsearch/http/nio/NioHttpChannel.java | 2 +- .../transport/nio/TcpNioServerSocketChannel.java | 2 +- .../elasticsearch/transport/nio/TcpNioSocketChannel.java | 2 +- .../org/elasticsearch/transport/nio/MockNioTransport.java | 8 +++----- 7 files changed, 7 insertions(+), 16 deletions(-) diff --git a/libs/nio/src/main/java/org/elasticsearch/nio/NioChannel.java b/libs/nio/src/main/java/org/elasticsearch/nio/NioChannel.java index cc34570dae61e..56e9bb89511f1 100644 --- a/libs/nio/src/main/java/org/elasticsearch/nio/NioChannel.java +++ b/libs/nio/src/main/java/org/elasticsearch/nio/NioChannel.java @@ -32,13 +32,8 @@ */ public abstract class NioChannel { - private final NetworkChannel socketChannel; private volatile InetSocketAddress localAddress; - NioChannel(NetworkChannel socketChannel) { - this.socketChannel = socketChannel; - } - public boolean isOpen() { return getContext().isOpen(); } @@ -46,7 +41,7 @@ public boolean isOpen() { public InetSocketAddress getLocalAddress() { if (localAddress == null) { try { - localAddress = (InetSocketAddress) socketChannel.getLocalAddress(); + localAddress = (InetSocketAddress) getRawChannel().getLocalAddress(); } catch (IOException e) {} } return localAddress; diff --git a/libs/nio/src/main/java/org/elasticsearch/nio/NioServerSocketChannel.java b/libs/nio/src/main/java/org/elasticsearch/nio/NioServerSocketChannel.java index 6e8a42db60ba7..3775939c0cb2c 100644 --- a/libs/nio/src/main/java/org/elasticsearch/nio/NioServerSocketChannel.java +++ b/libs/nio/src/main/java/org/elasticsearch/nio/NioServerSocketChannel.java @@ -29,7 +29,6 @@ public class NioServerSocketChannel extends NioChannel { private ServerChannelContext context; public NioServerSocketChannel(ServerSocketChannel socketChannel) { - super(socketChannel); this.socketChannel = socketChannel; } diff --git a/libs/nio/src/main/java/org/elasticsearch/nio/NioSocketChannel.java b/libs/nio/src/main/java/org/elasticsearch/nio/NioSocketChannel.java index 36e7dbc7aaa02..0496386916006 100644 --- a/libs/nio/src/main/java/org/elasticsearch/nio/NioSocketChannel.java +++ b/libs/nio/src/main/java/org/elasticsearch/nio/NioSocketChannel.java @@ -33,7 +33,6 @@ public class NioSocketChannel extends NioChannel { private SocketChannelContext context; public NioSocketChannel(SocketChannel socketChannel) { - super(socketChannel); this.socketChannel = socketChannel; } diff --git a/plugins/transport-nio/src/main/java/org/elasticsearch/http/nio/NioHttpChannel.java b/plugins/transport-nio/src/main/java/org/elasticsearch/http/nio/NioHttpChannel.java index 088f0e85dde23..3672bd4bde0ab 100644 --- a/plugins/transport-nio/src/main/java/org/elasticsearch/http/nio/NioHttpChannel.java +++ b/plugins/transport-nio/src/main/java/org/elasticsearch/http/nio/NioHttpChannel.java @@ -29,7 +29,7 @@ public class NioHttpChannel extends NioSocketChannel implements HttpChannel { - NioHttpChannel(SocketChannel socketChannel) throws IOException { + NioHttpChannel(SocketChannel socketChannel) { super(socketChannel); } diff --git a/plugins/transport-nio/src/main/java/org/elasticsearch/transport/nio/TcpNioServerSocketChannel.java b/plugins/transport-nio/src/main/java/org/elasticsearch/transport/nio/TcpNioServerSocketChannel.java index 946563225c66c..1792eb09c9346 100644 --- a/plugins/transport-nio/src/main/java/org/elasticsearch/transport/nio/TcpNioServerSocketChannel.java +++ b/plugins/transport-nio/src/main/java/org/elasticsearch/transport/nio/TcpNioServerSocketChannel.java @@ -36,7 +36,7 @@ public class TcpNioServerSocketChannel extends NioServerSocketChannel implements private final String profile; - public TcpNioServerSocketChannel(String profile, ServerSocketChannel socketChannel) throws IOException { + public TcpNioServerSocketChannel(String profile, ServerSocketChannel socketChannel) { super(socketChannel); this.profile = profile; } diff --git a/plugins/transport-nio/src/main/java/org/elasticsearch/transport/nio/TcpNioSocketChannel.java b/plugins/transport-nio/src/main/java/org/elasticsearch/transport/nio/TcpNioSocketChannel.java index ef2bc875aa994..6fc63c175f504 100644 --- a/plugins/transport-nio/src/main/java/org/elasticsearch/transport/nio/TcpNioSocketChannel.java +++ b/plugins/transport-nio/src/main/java/org/elasticsearch/transport/nio/TcpNioSocketChannel.java @@ -32,7 +32,7 @@ public class TcpNioSocketChannel extends NioSocketChannel implements TcpChannel private final String profile; - public TcpNioSocketChannel(String profile, SocketChannel socketChannel) throws IOException { + public TcpNioSocketChannel(String profile, SocketChannel socketChannel) { super(socketChannel); this.profile = profile; } diff --git a/test/framework/src/main/java/org/elasticsearch/transport/nio/MockNioTransport.java b/test/framework/src/main/java/org/elasticsearch/transport/nio/MockNioTransport.java index cb9e243660a8e..912c13e7fc612 100644 --- a/test/framework/src/main/java/org/elasticsearch/transport/nio/MockNioTransport.java +++ b/test/framework/src/main/java/org/elasticsearch/transport/nio/MockNioTransport.java @@ -164,7 +164,7 @@ public MockSocketChannel createChannel(NioSelector selector, SocketChannel chann @Override public MockServerChannel createServerChannel(NioSelector selector, ServerSocketChannel channel) throws IOException { - MockServerChannel nioServerChannel = new MockServerChannel(profileName, channel, this, selector); + MockServerChannel nioServerChannel = new MockServerChannel(profileName, channel); Consumer exceptionHandler = (e) -> logger.error(() -> new ParameterizedMessage("exception from server channel caught on transport layer [{}]", channel), e); ServerChannelContext context = new ServerChannelContext(nioServerChannel, this, selector, MockNioTransport.this::acceptChannel, @@ -195,8 +195,7 @@ private static class MockServerChannel extends NioServerSocketChannel implements private final String profile; - MockServerChannel(String profile, ServerSocketChannel channel, ChannelFactory channelFactory, NioSelector selector) - throws IOException { + MockServerChannel(String profile, ServerSocketChannel channel) { super(channel); this.profile = profile; } @@ -236,8 +235,7 @@ private static class MockSocketChannel extends NioSocketChannel implements TcpCh private final String profile; - private MockSocketChannel(String profile, java.nio.channels.SocketChannel socketChannel, NioSelector selector) - throws IOException { + private MockSocketChannel(String profile, java.nio.channels.SocketChannel socketChannel, NioSelector selector) { super(socketChannel); this.profile = profile; } From ca09a531051b447f08dc9e0357ea2aa9e74ae362 Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Tue, 19 Jun 2018 12:05:47 -0600 Subject: [PATCH 3/9] Cleanups --- .../src/main/java/org/elasticsearch/nio/NioChannel.java | 4 +++- .../org/elasticsearch/nio/NioServerSocketChannel.java | 4 ++++ .../main/java/org/elasticsearch/nio/NioSocketChannel.java | 8 ++++++-- .../java/org/elasticsearch/nio/SocketChannelContext.java | 3 +++ .../java/org/elasticsearch/http/nio/NioHttpChannel.java | 1 - 5 files changed, 16 insertions(+), 4 deletions(-) diff --git a/libs/nio/src/main/java/org/elasticsearch/nio/NioChannel.java b/libs/nio/src/main/java/org/elasticsearch/nio/NioChannel.java index 56e9bb89511f1..34b4424a84492 100644 --- a/libs/nio/src/main/java/org/elasticsearch/nio/NioChannel.java +++ b/libs/nio/src/main/java/org/elasticsearch/nio/NioChannel.java @@ -42,7 +42,9 @@ public InetSocketAddress getLocalAddress() { if (localAddress == null) { try { localAddress = (InetSocketAddress) getRawChannel().getLocalAddress(); - } catch (IOException e) {} + } catch (IOException e) { + // We are not care about this exception. + } } return localAddress; } diff --git a/libs/nio/src/main/java/org/elasticsearch/nio/NioServerSocketChannel.java b/libs/nio/src/main/java/org/elasticsearch/nio/NioServerSocketChannel.java index 3775939c0cb2c..b2d12f590c0d9 100644 --- a/libs/nio/src/main/java/org/elasticsearch/nio/NioServerSocketChannel.java +++ b/libs/nio/src/main/java/org/elasticsearch/nio/NioServerSocketChannel.java @@ -30,6 +30,10 @@ public class NioServerSocketChannel extends NioChannel { public NioServerSocketChannel(ServerSocketChannel socketChannel) { this.socketChannel = socketChannel; + // Calling getLocalAddress will attempt to set the local address for future calls. This only happens + // if the channel is bound (which is the case for server channels but likely not the case for socket + // channels). + getLocalAddress(); } /** diff --git a/libs/nio/src/main/java/org/elasticsearch/nio/NioSocketChannel.java b/libs/nio/src/main/java/org/elasticsearch/nio/NioSocketChannel.java index 0496386916006..73765cae71aa1 100644 --- a/libs/nio/src/main/java/org/elasticsearch/nio/NioSocketChannel.java +++ b/libs/nio/src/main/java/org/elasticsearch/nio/NioSocketChannel.java @@ -34,6 +34,8 @@ public class NioSocketChannel extends NioChannel { public NioSocketChannel(SocketChannel socketChannel) { this.socketChannel = socketChannel; + // Calling getRemoteAddress will attempt to set the local address for future calls. + getRemoteAddress(); } public void setContext(SocketChannelContext context) { @@ -57,8 +59,10 @@ public SocketChannelContext getContext() { public InetSocketAddress getRemoteAddress() { if (remoteAddress == null) { try { - remoteAddress = (InetSocketAddress) socketChannel.getLocalAddress(); - } catch (IOException e) {} + remoteAddress = (InetSocketAddress) socketChannel.getRemoteAddress(); + } catch (IOException e) { + // We are not care about this exception. + } } return remoteAddress; } diff --git a/libs/nio/src/main/java/org/elasticsearch/nio/SocketChannelContext.java b/libs/nio/src/main/java/org/elasticsearch/nio/SocketChannelContext.java index 53fb0da432f48..fcc5332b69587 100644 --- a/libs/nio/src/main/java/org/elasticsearch/nio/SocketChannelContext.java +++ b/libs/nio/src/main/java/org/elasticsearch/nio/SocketChannelContext.java @@ -117,6 +117,9 @@ public boolean connect() throws IOException { } if (isConnected) { connectContext.complete(null); + // Calling getLocalAddress will attempt to set the local address for future calls. We are calling + // it here as the channel should have a local address once the connection is complete. + channel.getLocalAddress(); } return isConnected; } diff --git a/plugins/transport-nio/src/main/java/org/elasticsearch/http/nio/NioHttpChannel.java b/plugins/transport-nio/src/main/java/org/elasticsearch/http/nio/NioHttpChannel.java index 8aa4c2b1a77b7..0a797a5687ec7 100644 --- a/plugins/transport-nio/src/main/java/org/elasticsearch/http/nio/NioHttpChannel.java +++ b/plugins/transport-nio/src/main/java/org/elasticsearch/http/nio/NioHttpChannel.java @@ -24,7 +24,6 @@ import org.elasticsearch.http.HttpResponse; import org.elasticsearch.nio.NioSocketChannel; -import java.io.IOException; import java.nio.channels.SocketChannel; public class NioHttpChannel extends NioSocketChannel implements HttpChannel { From a2a287418cd1ae37c0c01a318fe79191f58fae94 Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Tue, 19 Jun 2018 12:28:20 -0600 Subject: [PATCH 4/9] Some more cleanups --- .../org/elasticsearch/nio/NioChannel.java | 16 ++--------- .../nio/NioServerSocketChannel.java | 22 +++++++++------ .../elasticsearch/nio/NioSocketChannel.java | 27 ++++++++++++++----- .../nio/SocketChannelContext.java | 3 --- 4 files changed, 37 insertions(+), 31 deletions(-) diff --git a/libs/nio/src/main/java/org/elasticsearch/nio/NioChannel.java b/libs/nio/src/main/java/org/elasticsearch/nio/NioChannel.java index 34b4424a84492..55038fabcef8e 100644 --- a/libs/nio/src/main/java/org/elasticsearch/nio/NioChannel.java +++ b/libs/nio/src/main/java/org/elasticsearch/nio/NioChannel.java @@ -19,7 +19,6 @@ package org.elasticsearch.nio; -import java.io.IOException; import java.net.InetSocketAddress; import java.nio.channels.NetworkChannel; import java.util.function.BiConsumer; @@ -32,23 +31,10 @@ */ public abstract class NioChannel { - private volatile InetSocketAddress localAddress; - public boolean isOpen() { return getContext().isOpen(); } - public InetSocketAddress getLocalAddress() { - if (localAddress == null) { - try { - localAddress = (InetSocketAddress) getRawChannel().getLocalAddress(); - } catch (IOException e) { - // We are not care about this exception. - } - } - return localAddress; - } - /** * Adds a close listener to the channel. Multiple close listeners can be added. There is no guarantee * about the order in which close listeners will be executed. If the channel is already closed, the @@ -67,6 +53,8 @@ public void close() { getContext().closeChannel(); } + public abstract InetSocketAddress getLocalAddress(); + public abstract NetworkChannel getRawChannel(); public abstract ChannelContext getContext(); diff --git a/libs/nio/src/main/java/org/elasticsearch/nio/NioServerSocketChannel.java b/libs/nio/src/main/java/org/elasticsearch/nio/NioServerSocketChannel.java index b2d12f590c0d9..b7b80b4d490e6 100644 --- a/libs/nio/src/main/java/org/elasticsearch/nio/NioServerSocketChannel.java +++ b/libs/nio/src/main/java/org/elasticsearch/nio/NioServerSocketChannel.java @@ -19,21 +19,19 @@ package org.elasticsearch.nio; +import java.net.InetSocketAddress; import java.nio.channels.ServerSocketChannel; import java.util.concurrent.atomic.AtomicBoolean; public class NioServerSocketChannel extends NioChannel { - private final ServerSocketChannel socketChannel; + private final ServerSocketChannel serverSocketChannel; private final AtomicBoolean contextSet = new AtomicBoolean(false); + private volatile InetSocketAddress localAddress; private ServerChannelContext context; - public NioServerSocketChannel(ServerSocketChannel socketChannel) { - this.socketChannel = socketChannel; - // Calling getLocalAddress will attempt to set the local address for future calls. This only happens - // if the channel is bound (which is the case for server channels but likely not the case for socket - // channels). - getLocalAddress(); + public NioServerSocketChannel(ServerSocketChannel serverSocketChannel) { + this.serverSocketChannel = serverSocketChannel; } /** @@ -50,9 +48,17 @@ public void setContext(ServerChannelContext context) { } } + @Override + public InetSocketAddress getLocalAddress() { + if (localAddress == null) { + localAddress = (InetSocketAddress) serverSocketChannel.socket().getLocalSocketAddress(); + } + return localAddress; + } + @Override public ServerSocketChannel getRawChannel() { - return socketChannel; + return serverSocketChannel; } @Override diff --git a/libs/nio/src/main/java/org/elasticsearch/nio/NioSocketChannel.java b/libs/nio/src/main/java/org/elasticsearch/nio/NioSocketChannel.java index 73765cae71aa1..9a5eccb4a7645 100644 --- a/libs/nio/src/main/java/org/elasticsearch/nio/NioSocketChannel.java +++ b/libs/nio/src/main/java/org/elasticsearch/nio/NioSocketChannel.java @@ -21,6 +21,7 @@ import java.io.IOException; import java.net.InetSocketAddress; +import java.net.Socket; import java.nio.channels.SocketChannel; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.BiConsumer; @@ -30,12 +31,11 @@ public class NioSocketChannel extends NioChannel { private final AtomicBoolean contextSet = new AtomicBoolean(false); private final SocketChannel socketChannel; private volatile InetSocketAddress remoteAddress; + private volatile InetSocketAddress localAddress; private SocketChannelContext context; public NioSocketChannel(SocketChannel socketChannel) { this.socketChannel = socketChannel; - // Calling getRemoteAddress will attempt to set the local address for future calls. - getRemoteAddress(); } public void setContext(SocketChannelContext context) { @@ -46,6 +46,14 @@ public void setContext(SocketChannelContext context) { } } + @Override + public InetSocketAddress getLocalAddress() { + if (localAddress == null) { + localAddress = (InetSocketAddress) socketChannel.socket().getLocalSocketAddress(); + } + return localAddress; + } + @Override public SocketChannel getRawChannel() { return socketChannel; @@ -58,10 +66,17 @@ public SocketChannelContext getContext() { public InetSocketAddress getRemoteAddress() { if (remoteAddress == null) { - try { - remoteAddress = (InetSocketAddress) socketChannel.getRemoteAddress(); - } catch (IOException e) { - // We are not care about this exception. + Socket socket = socketChannel.socket(); + // socket.getRemoteSocketAddress() will only return the address if the socket is connected. + if (socket.isConnected()) { + remoteAddress = (InetSocketAddress) socket.getRemoteSocketAddress(); + } else { + try { + remoteAddress = (InetSocketAddress) socketChannel.getRemoteAddress(); + } catch (IOException e) { + // This exception will be thrown if the channel is closed. But we do not really care when + // we are just attempting to read the remote address. + } } } return remoteAddress; diff --git a/libs/nio/src/main/java/org/elasticsearch/nio/SocketChannelContext.java b/libs/nio/src/main/java/org/elasticsearch/nio/SocketChannelContext.java index fcc5332b69587..53fb0da432f48 100644 --- a/libs/nio/src/main/java/org/elasticsearch/nio/SocketChannelContext.java +++ b/libs/nio/src/main/java/org/elasticsearch/nio/SocketChannelContext.java @@ -117,9 +117,6 @@ public boolean connect() throws IOException { } if (isConnected) { connectContext.complete(null); - // Calling getLocalAddress will attempt to set the local address for future calls. We are calling - // it here as the channel should have a local address once the connection is complete. - channel.getLocalAddress(); } return isConnected; } From 20eca6ff4cfef4fa8a732b3cf86b69dd5de1d278 Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Tue, 19 Jun 2018 12:37:11 -0600 Subject: [PATCH 5/9] Initialize remote address in constructor --- .../elasticsearch/nio/NioSocketChannel.java | 21 ++++++------------- 1 file changed, 6 insertions(+), 15 deletions(-) diff --git a/libs/nio/src/main/java/org/elasticsearch/nio/NioSocketChannel.java b/libs/nio/src/main/java/org/elasticsearch/nio/NioSocketChannel.java index 9a5eccb4a7645..bf79231c7a0ee 100644 --- a/libs/nio/src/main/java/org/elasticsearch/nio/NioSocketChannel.java +++ b/libs/nio/src/main/java/org/elasticsearch/nio/NioSocketChannel.java @@ -21,7 +21,6 @@ import java.io.IOException; import java.net.InetSocketAddress; -import java.net.Socket; import java.nio.channels.SocketChannel; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.BiConsumer; @@ -36,6 +35,12 @@ public class NioSocketChannel extends NioChannel { public NioSocketChannel(SocketChannel socketChannel) { this.socketChannel = socketChannel; + try { + remoteAddress = (InetSocketAddress) socketChannel.getRemoteAddress(); + } catch (IOException e) { + // This exception will be thrown if the channel is closed. But we do not really care when + // we are just attempting to read the remote address. + } } public void setContext(SocketChannelContext context) { @@ -65,20 +70,6 @@ public SocketChannelContext getContext() { } public InetSocketAddress getRemoteAddress() { - if (remoteAddress == null) { - Socket socket = socketChannel.socket(); - // socket.getRemoteSocketAddress() will only return the address if the socket is connected. - if (socket.isConnected()) { - remoteAddress = (InetSocketAddress) socket.getRemoteSocketAddress(); - } else { - try { - remoteAddress = (InetSocketAddress) socketChannel.getRemoteAddress(); - } catch (IOException e) { - // This exception will be thrown if the channel is closed. But we do not really care when - // we are just attempting to read the remote address. - } - } - } return remoteAddress; } From e1caed35ba5feee6eb970c3c0a9c1d2a40d1b37a Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Wed, 20 Jun 2018 08:45:14 -0600 Subject: [PATCH 6/9] Throw as unchecked --- .../main/java/org/elasticsearch/nio/NioSocketChannel.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/libs/nio/src/main/java/org/elasticsearch/nio/NioSocketChannel.java b/libs/nio/src/main/java/org/elasticsearch/nio/NioSocketChannel.java index bf79231c7a0ee..c7d44990837cd 100644 --- a/libs/nio/src/main/java/org/elasticsearch/nio/NioSocketChannel.java +++ b/libs/nio/src/main/java/org/elasticsearch/nio/NioSocketChannel.java @@ -20,6 +20,7 @@ package org.elasticsearch.nio; import java.io.IOException; +import java.io.UncheckedIOException; import java.net.InetSocketAddress; import java.nio.channels.SocketChannel; import java.util.concurrent.atomic.AtomicBoolean; @@ -29,17 +30,16 @@ public class NioSocketChannel extends NioChannel { private final AtomicBoolean contextSet = new AtomicBoolean(false); private final SocketChannel socketChannel; - private volatile InetSocketAddress remoteAddress; + private final InetSocketAddress remoteAddress; private volatile InetSocketAddress localAddress; private SocketChannelContext context; public NioSocketChannel(SocketChannel socketChannel) { this.socketChannel = socketChannel; try { - remoteAddress = (InetSocketAddress) socketChannel.getRemoteAddress(); + this.remoteAddress = (InetSocketAddress) socketChannel.getRemoteAddress(); } catch (IOException e) { - // This exception will be thrown if the channel is closed. But we do not really care when - // we are just attempting to read the remote address. + throw new UncheckedIOException(e); } } From 58aba139735ea657fcf506ec9f8b7f0444295085 Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Wed, 20 Jun 2018 08:59:54 -0600 Subject: [PATCH 7/9] Repropogate unchecked ioexception --- .../src/main/java/org/elasticsearch/nio/ChannelFactory.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/libs/nio/src/main/java/org/elasticsearch/nio/ChannelFactory.java b/libs/nio/src/main/java/org/elasticsearch/nio/ChannelFactory.java index 77443d948d9a6..f2075133e3e99 100644 --- a/libs/nio/src/main/java/org/elasticsearch/nio/ChannelFactory.java +++ b/libs/nio/src/main/java/org/elasticsearch/nio/ChannelFactory.java @@ -21,6 +21,7 @@ import java.io.Closeable; import java.io.IOException; +import java.io.UncheckedIOException; import java.net.InetSocketAddress; import java.nio.channels.ServerSocketChannel; import java.nio.channels.SocketChannel; @@ -99,6 +100,10 @@ private Socket internalCreateChannel(NioSelector selector, SocketChannel rawChan Socket channel = createChannel(selector, rawChannel); assert channel.getContext() != null : "channel context should have been set on channel"; return channel; + } catch (UncheckedIOException e) { + // This can happen if getRemoteAddress throws IOException. + closeRawChannel(rawChannel, e); + throw e.getCause(); } catch (Exception e) { closeRawChannel(rawChannel, e); throw e; From a0599db1134b17c9c9e0b6e16800c93bee27b192 Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Wed, 20 Jun 2018 09:04:36 -0600 Subject: [PATCH 8/9] Better propogate --- .../src/main/java/org/elasticsearch/nio/ChannelFactory.java | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/libs/nio/src/main/java/org/elasticsearch/nio/ChannelFactory.java b/libs/nio/src/main/java/org/elasticsearch/nio/ChannelFactory.java index f2075133e3e99..f0dc3e567fef6 100644 --- a/libs/nio/src/main/java/org/elasticsearch/nio/ChannelFactory.java +++ b/libs/nio/src/main/java/org/elasticsearch/nio/ChannelFactory.java @@ -102,8 +102,9 @@ private Socket internalCreateChannel(NioSelector selector, SocketChannel rawChan return channel; } catch (UncheckedIOException e) { // This can happen if getRemoteAddress throws IOException. - closeRawChannel(rawChannel, e); - throw e.getCause(); + IOException cause = e.getCause(); + closeRawChannel(rawChannel, cause); + throw cause; } catch (Exception e) { closeRawChannel(rawChannel, e); throw e; From 19498fd94a0856a5948891f19b3ece67afeb6a4b Mon Sep 17 00:00:00 2001 From: Tim Brooks Date: Wed, 20 Jun 2018 17:33:07 -0600 Subject: [PATCH 9/9] Fix tests --- .../test/java/org/elasticsearch/nio/EventHandlerTests.java | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/libs/nio/src/test/java/org/elasticsearch/nio/EventHandlerTests.java b/libs/nio/src/test/java/org/elasticsearch/nio/EventHandlerTests.java index a9e1836199e25..0cc3aa048008a 100644 --- a/libs/nio/src/test/java/org/elasticsearch/nio/EventHandlerTests.java +++ b/libs/nio/src/test/java/org/elasticsearch/nio/EventHandlerTests.java @@ -23,6 +23,7 @@ import org.junit.Before; import java.io.IOException; +import java.net.ServerSocket; import java.nio.channels.CancelledKeyException; import java.nio.channels.SelectionKey; import java.nio.channels.ServerSocketChannel; @@ -69,7 +70,9 @@ public void setUpHandler() throws IOException { channel.setContext(context); handler.handleRegistration(context); - NioServerSocketChannel serverChannel = new NioServerSocketChannel(mock(ServerSocketChannel.class)); + ServerSocketChannel serverSocketChannel = mock(ServerSocketChannel.class); + when(serverSocketChannel.socket()).thenReturn(mock(ServerSocket.class)); + NioServerSocketChannel serverChannel = new NioServerSocketChannel(serverSocketChannel); serverContext = new DoNotRegisterServerContext(serverChannel, mock(NioSelector.class), mock(Consumer.class)); serverChannel.setContext(serverContext);