zentol commented on a change in pull request #17227: URL: https://github.com/apache/flink/pull/17227#discussion_r705654583
########## File path: flink-core/src/main/java/org/apache/flink/util/NetUtils.java ########## @@ -116,6 +119,39 @@ private static URL validateHostPortString(String hostPort) { } } + /** + * Calls {@link ServerSocket#accept()} on the provided server socket, suppressing any thrown + * {@link SocketTimeoutException}s. This is a workaround for the underlying JDK-8237858 bug in + * JDK 11 that can cause errant SocketTimeoutExceptions to be thrown at unexpected times. + * + * <p>This method expects the provided ServerSocket has no timeout set (SO_TIMEOUT of 0), + * indicating an infinite timeout. It will suppress all SocketTimeoutExceptions, even if a + * ServerSocket with a non-zero timeout is passed in. + * + * @param serverSocket a ServerSocket with {@link SocketOptions#SO_TIMEOUT SO_TIMEOUT} set to 0; + * if SO_TIMEOUT is greater than 0, then this method will suppress SocketTimeoutException; + * must not be null + * @return the new Socket + * @exception IOException if an I/O error occurs when waiting for a connection. + * @exception SecurityException if a security manager exists and its {@code checkAccept} method Review comment: We shouldn't copy code from the JDK verbatim. Let's just add a reference to `ServerSocket#accept`. ########## File path: flink-core/src/main/java/org/apache/flink/util/NetUtils.java ########## @@ -116,6 +119,39 @@ private static URL validateHostPortString(String hostPort) { } } + /** + * Calls {@link ServerSocket#accept()} on the provided server socket, suppressing any thrown + * {@link SocketTimeoutException}s. This is a workaround for the underlying JDK-8237858 bug in + * JDK 11 that can cause errant SocketTimeoutExceptions to be thrown at unexpected times. + * + * <p>This method expects the provided ServerSocket has no timeout set (SO_TIMEOUT of 0), + * indicating an infinite timeout. It will suppress all SocketTimeoutExceptions, even if a + * ServerSocket with a non-zero timeout is passed in. + * + * @param serverSocket a ServerSocket with {@link SocketOptions#SO_TIMEOUT SO_TIMEOUT} set to 0; + * if SO_TIMEOUT is greater than 0, then this method will suppress SocketTimeoutException; Review comment: Could we not reject such sockets via `ServerSocket#getSoTimeout`? ########## File path: flink-core/src/test/java/org/apache/flink/util/NetUtilsTest.java ########## @@ -49,6 +58,30 @@ public void testParseHostPortAddress() { assertEquals(socketAddress, NetUtils.parseHostPortAddress("foo.com:8080")); } + @Test + public void testAcceptWithoutTimeout() throws IOException { + // Validates that acceptWithoutTimeout suppresses all SocketTimeoutExceptions + ServerSocket serverSocket = mock(ServerSocket.class); Review comment: Instead of relying on mockito which we very much want to avoid, we instead can create a `ServerSocket` subclass and override `accept()` accordingly. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@flink.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org