This is an automated email from the ASF dual-hosted git repository.
gosullivan pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode.git
The following commit(s) were added to refs/heads/develop by this push:
new b55215d GEODE-3563: use a timeout for newly created sockets in
TcpConduit.run() (#1671)
b55215d is described below
commit b55215dcb64c86554d531b0b475e85f013e76fc6
Author: Galen O'Sullivan <gosulli...@pivotal.io>
AuthorDate: Fri Mar 30 11:13:15 2018 -0700
GEODE-3563: use a timeout for newly created sockets in TcpConduit.run()
(#1671)
* GEODE-3563: use a timeout in SocketCreator.ConfigureServerSSLSocket()
Also close newly accepted sockets in TcpConduit.run() if SSL
configuration fails (or any other IOException).
* Add units.
* Rename to startHandshakeIfSocketIsSSL for clarity.
---
.../distributed/internal/tcpserver/TcpServer.java | 5 +--
.../internal/cache/tier/sockets/AcceptorImpl.java | 3 +-
.../apache/geode/internal/net/SocketCreator.java | 7 +++-
.../org/apache/geode/internal/tcp/TCPConduit.java | 37 +++++++++++++---------
.../geode/internal/net/DummySocketCreator.java | 8 ++---
.../internal/net/SSLSocketIntegrationTest.java | 5 +--
.../geode/internal/net/SocketCreatorJUnitTest.java | 17 ++++++++++
7 files changed, 52 insertions(+), 30 deletions(-)
diff --git
a/geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpServer.java
b/geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpServer.java
index 169b09d..778e3d9 100755
---
a/geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpServer.java
+++
b/geode-core/src/main/java/org/apache/geode/distributed/internal/tcpserver/TcpServer.java
@@ -345,7 +345,6 @@ public class TcpServer {
}
handler.shutDown();
synchronized (this) {
- // this.shutDown = true;
this.notifyAll();
}
}
@@ -360,9 +359,7 @@ public class TcpServer {
long startTime = DistributionStats.getStatTime();
DataInputStream input = null;
try {
-
- socket.setSoTimeout(READ_TIMEOUT);
- getSocketCreator().configureServerSSLSocket(socket);
+ getSocketCreator().startHandshakeIfSocketIsSSL(socket, READ_TIMEOUT);
try {
input = new DataInputStream(socket.getInputStream());
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
index d060ed2..83d3156 100755
---
a/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
+++
b/geode-core/src/main/java/org/apache/geode/internal/cache/tier/sockets/AcceptorImpl.java
@@ -1549,8 +1549,7 @@ public class AcceptorImpl implements Acceptor, Runnable,
CommBufferPool {
}
private CommunicationMode getCommunicationModeForNonSelector(Socket socket)
throws IOException {
- socket.setSoTimeout(this.acceptTimeout);
- this.socketCreator.configureServerSSLSocket(socket);
+ this.socketCreator.startHandshakeIfSocketIsSSL(socket, this.acceptTimeout);
byte communicationModeByte = (byte) socket.getInputStream().read();
if (communicationModeByte == -1) {
throw new EOFException();
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/net/SocketCreator.java
b/geode-core/src/main/java/org/apache/geode/internal/net/SocketCreator.java
index 9478953..31456c0 100755
--- a/geode-core/src/main/java/org/apache/geode/internal/net/SocketCreator.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/net/SocketCreator.java
@@ -968,8 +968,12 @@ public class SocketCreator {
/**
* Will be a server socket... this one simply registers the listeners.
+ *
+ * @param timeout the socket's timeout will be set to this (in milliseconds).
*/
- public void configureServerSSLSocket(Socket socket) throws IOException {
+ public void startHandshakeIfSocketIsSSL(Socket socket, int timeout) throws
IOException {
+ socket.setSoTimeout(timeout);
+
if (socket instanceof SSLSocket) {
SSLSocket sslSocket = (SSLSocket) socket;
try {
@@ -990,6 +994,7 @@ public class SocketCreator {
ex);
throw ex;
}
+ // else ignore
}
}
}
diff --git
a/geode-core/src/main/java/org/apache/geode/internal/tcp/TCPConduit.java
b/geode-core/src/main/java/org/apache/geode/internal/tcp/TCPConduit.java
index 9483e08..5487039 100644
--- a/geode-core/src/main/java/org/apache/geode/internal/tcp/TCPConduit.java
+++ b/geode-core/src/main/java/org/apache/geode/internal/tcp/TCPConduit.java
@@ -598,9 +598,8 @@ public class TCPConduit implements Runnable {
LocalizedStrings.TCPConduit_UNABLE_TO_SHUT_DOWN_LISTENER_WITHIN_0_MS_UNABLE_TO_INTERRUPT_SOCKET_ACCEPT_DUE_TO_JDK_BUG_GIVING_UP,
Integer.valueOf(LISTENER_CLOSE_TIMEOUT)));
}
- } catch (IOException e) {
- } catch (InterruptedException e) {
- // Ignore, we're trying to stop already.
+ } catch (IOException | InterruptedException e) {
+ // we're already trying to shutdown, ignore
} finally {
this.hsPool.shutdownNow();
}
@@ -689,7 +688,7 @@ public class TCPConduit implements Runnable {
ex);
break;
}
- socketCreator.configureServerSSLSocket(othersock);
+ socketCreator.startHandshakeIfSocketIsSSL(othersock,
idleConnectionTimeout);
}
if (stopped) {
try {
@@ -705,11 +704,19 @@ public class TCPConduit implements Runnable {
} catch (ClosedByInterruptException cbie) {
// safe to ignore
- } catch (ClosedChannelException e) {
- break; // we're dead
- } catch (CancelException e) {
+ } catch (ClosedChannelException | CancelException e) {
break;
- } catch (Exception e) {
+ } catch (IOException e) {
+ this.getStats().incFailedAccept();
+
+ try {
+ if (othersock != null) {
+ othersock.close();
+ }
+ } catch (IOException ignore) {
+
+ }
+
if (!stopped) {
if (e instanceof SocketException && "Socket
closed".equalsIgnoreCase(e.getMessage())) {
// safe to ignore; see bug 31156
@@ -737,17 +744,17 @@ public class TCPConduit implements Runnable {
}
}
}
+ } else if ("Too many open files".equals(e.getMessage())) {
+ getConTable().fileDescriptorsExhausted();
} else {
- this.getStats().incFailedAccept();
- if (e instanceof IOException && "Too many open
files".equals(e.getMessage())) {
- getConTable().fileDescriptorsExhausted();
- } else {
- logger.warn(e.getMessage(), e);
- }
+ logger.warn(e.getMessage(), e);
}
+
}
- // connections.cleanupLowWater();
+ } catch (Exception e) {
+ logger.warn(e.getMessage(), e);
}
+
if (!stopped && socket.isClosed()) {
// NOTE: do not check for distributed system closing here. Messaging
// may need to occur during the closing of the DS or cache
diff --git
a/geode-core/src/test/java/org/apache/geode/internal/net/DummySocketCreator.java
b/geode-core/src/test/java/org/apache/geode/internal/net/DummySocketCreator.java
index 6928b6f..e061f14 100644
---
a/geode-core/src/test/java/org/apache/geode/internal/net/DummySocketCreator.java
+++
b/geode-core/src/test/java/org/apache/geode/internal/net/DummySocketCreator.java
@@ -35,12 +35,8 @@ public class DummySocketCreator extends SocketCreator {
}
@Override
- public void configureServerSSLSocket(Socket socket) throws IOException {
- this.socketSoTimeouts.add(socket.getSoTimeout());
+ public void startHandshakeIfSocketIsSSL(Socket socket, int timeout) throws
IOException {
+ this.socketSoTimeouts.add(timeout);
throw new SSLException("This is a test SSLException");
}
-
- public List<Integer> getSocketSoTimeouts() {
- return socketSoTimeouts;
- }
}
diff --git
a/geode-core/src/test/java/org/apache/geode/internal/net/SSLSocketIntegrationTest.java
b/geode-core/src/test/java/org/apache/geode/internal/net/SSLSocketIntegrationTest.java
index 704beb6..af06241 100755
---
a/geode-core/src/test/java/org/apache/geode/internal/net/SSLSocketIntegrationTest.java
+++
b/geode-core/src/test/java/org/apache/geode/internal/net/SSLSocketIntegrationTest.java
@@ -209,7 +209,7 @@ public class SSLSocketIntegrationTest {
Awaitility.await("connect to server socket").atMost(30,
TimeUnit.SECONDS).until(() -> {
try {
Socket clientSocket = socketCreator.connectForClient(
- SocketCreator.getLocalHost().getHostAddress(), serverSocketPort,
2000);
+ SocketCreator.getLocalHost().getHostAddress(), serverSocketPort,
500);
clientSocket.close();
System.err.println(
"client successfully connected to server but should not have
been able to do so");
@@ -254,7 +254,8 @@ public class SSLSocketIntegrationTest {
Thread serverThread = new Thread(new
MyThreadGroup(this.testName.getMethodName()), () -> {
try {
Socket socket = serverSocket.accept();
-
SocketCreatorFactory.getSocketCreatorForComponent(CLUSTER).configureServerSSLSocket(socket);
+ SocketCreatorFactory.getSocketCreatorForComponent(CLUSTER)
+ .startHandshakeIfSocketIsSSL(socket, 15000);
ObjectInputStream ois = new ObjectInputStream(socket.getInputStream());
messageFromClient.set((String) ois.readObject());
} catch (IOException | ClassNotFoundException e) {
diff --git
a/geode-core/src/test/java/org/apache/geode/internal/net/SocketCreatorJUnitTest.java
b/geode-core/src/test/java/org/apache/geode/internal/net/SocketCreatorJUnitTest.java
index 2fcd7f6..33d34bb 100644
---
a/geode-core/src/test/java/org/apache/geode/internal/net/SocketCreatorJUnitTest.java
+++
b/geode-core/src/test/java/org/apache/geode/internal/net/SocketCreatorJUnitTest.java
@@ -14,6 +14,12 @@
*/
package org.apache.geode.internal.net;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+
+import java.net.Socket;
+
import org.junit.Test;
import org.junit.experimental.categories.Category;
@@ -37,6 +43,17 @@ public class SocketCreatorJUnitTest {
}
+ @Test
+ public void testConfigureServerSSLSocketSetsSoTimeout() throws Exception {
+ final SocketCreator socketCreator = new
SocketCreator(mock(SSLConfig.class));
+ final Socket socket = mock(Socket.class);
+
+ final int timeout = 1938236;
+ socketCreator.startHandshakeIfSocketIsSSL(socket, timeout);
+
+ verify(socket).setSoTimeout(timeout);
+ }
+
private String getSingleKeyKeystore() {
return TestUtil.getResourcePath(getClass(), "/ssl/trusted.keystore");
}
--
To stop receiving notification emails like this one, please contact
gosulli...@apache.org.
