THRIFT-896. java: TNonblockingSocket.isOpen() returns true even after close()
This patch makes TNonblockingSocket.isOpen() have more expected behavior.
Patch: Eric Jensen
git-svn-id: https://svn.apache.org/repos/asf/incubator/thrift/trunk@995939 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/lib/java/src/org/apache/thrift/transport/TNonblockingSocket.java b/lib/java/src/org/apache/thrift/transport/TNonblockingSocket.java
index c3787a2..482bd14 100644
--- a/lib/java/src/org/apache/thrift/transport/TNonblockingSocket.java
+++ b/lib/java/src/org/apache/thrift/transport/TNonblockingSocket.java
@@ -30,7 +30,6 @@
import java.nio.channels.Selector;
import java.nio.channels.SocketChannel;
-import org.apache.thrift.async.TAsyncMethodCall;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -44,12 +43,10 @@
/**
* Host and port if passed in, used for lazy non-blocking connect.
*/
- private SocketAddress socketAddress_ = null;
+ private final SocketAddress socketAddress_;
private final SocketChannel socketChannel_;
- private final Socket socket_;
-
public TNonblockingSocket(String host, int port) throws IOException {
this(host, port, 0);
}
@@ -62,8 +59,7 @@
* @throws IOException
*/
public TNonblockingSocket(String host, int port, int timeout) throws IOException {
- this(SocketChannel.open(), timeout);
- socketAddress_ = new InetSocketAddress(host, port);
+ this(SocketChannel.open(), timeout, new InetSocketAddress(host, port));
}
/**
@@ -73,18 +69,22 @@
* @throws IOException if there is an error setting up the streams
*/
public TNonblockingSocket(SocketChannel socketChannel) throws IOException {
- this(socketChannel, 0);
+ this(socketChannel, 0, null);
if (!socketChannel.isConnected()) throw new IOException("Socket must already be connected");
}
- private TNonblockingSocket(SocketChannel socketChannel, int timeout) throws IOException {
+ private TNonblockingSocket(SocketChannel socketChannel, int timeout, SocketAddress socketAddress)
+ throws IOException {
socketChannel_ = socketChannel;
- socket_ = socketChannel.socket();
+ socketAddress_ = socketAddress;
// make it a nonblocking channel
socketChannel.configureBlocking(false);
- socket_.setSoLinger(false, 0);
- socket_.setTcpNoDelay(true);
+
+ // set options
+ Socket socket = socketChannel.socket();
+ socket.setSoLinger(false, 0);
+ socket.setTcpNoDelay(true);
setTimeout(timeout);
}
@@ -106,24 +106,25 @@
*/
public void setTimeout(int timeout) {
try {
- socket_.setSoTimeout(timeout);
+ socketChannel_.socket().setSoTimeout(timeout);
} catch (SocketException sx) {
LOGGER.warn("Could not set socket timeout.", sx);
}
}
/**
- * Returns a reference to the underlying socket.
+ * Returns a reference to the underlying SocketChannel.
*/
- public Socket getSocket() {
- return socket_;
+ public SocketChannel getSocketChannel() {
+ return socketChannel_;
}
/**
* Checks whether the socket is connected.
*/
public boolean isOpen() {
- return socket_.isConnected();
+ // isConnected() does not return false after close(), but isOpen() does
+ return socketChannel_.isOpen() && socketChannel_.isConnected();
}
/**
diff --git a/lib/java/test/org/apache/thrift/async/TestTAsyncClientManager.java b/lib/java/test/org/apache/thrift/async/TestTAsyncClientManager.java
index 5e1084c..187c8f8 100644
--- a/lib/java/test/org/apache/thrift/async/TestTAsyncClientManager.java
+++ b/lib/java/test/org/apache/thrift/async/TestTAsyncClientManager.java
@@ -290,6 +290,7 @@
// check that timeouts work
assertFalse(s.isStopped());
+ assertTrue(clientSock.isOpen());
final CountDownLatch timeoutLatch = new CountDownLatch(1);
client.setTimeout(100);
client.primitiveMethod(new AsyncMethodCallback<primitiveMethod_call>() {
@@ -320,5 +321,8 @@
timeoutLatch.await(2, TimeUnit.SECONDS);
assertTrue(client.hasError());
assertTrue(client.getError() instanceof TimeoutException);
+
+ // error closes socket and make sure isOpen reflects that
+ assertFalse(clientSock.isOpen());
}
}