On Wed, 16 Aug 2023 23:11:11 GMT, Weibing Xiao <d...@openjdk.org> wrote:
>> Please refer to JDK-8314063. >> >> The failure scenario is due to the setting of connection timeout. It is >> either too small or not an optimal value for the system. When the client >> tries to connect to the server with LDAPs protocol. It requires the >> handshake after the socket is created and connected, but it fails due to >> connection timeout and leaves the socket open. It is not closed properly due >> to the exception handling in the JDK code. >> >> The change is adding a try/catch block and closing the socket in the catch >> block, and the format of the code got changed consequently. > > Weibing Xiao has updated the pull request incrementally with one additional > commit since the last revision: > > updated the code according to the review you now have two methods closing a socket. Can these be refactored and a single closeSocket method used in both cases ? method overloading could have been used for the new create socket method you might consider some restructuring of the call flow in each: private Socket createSocketWithoutFactory (String host, int port, int connectTimeout) throws Exception { Socket socket = null; InetSocketAddress endpoint = createInetSocketAddress(host, port); socket = new Socket(); if (connectTimeout > 0) { if (debug) { System.err.println("Connection: creating socket with " + "a timeout"); } socket.connect(endpoint, connectTimeout); } else { if (debug) { System.err.println("Connection: creating socket"); } // connected socket socket.connect(endpoint); } return socket; } // create the socket with the provided factory private Socket createSocketWithFactory(String host, int port, String socketFactory, int connectTimeout) throws Exception { Socket socket = null; @SuppressWarnings("unchecked") Class<? extends SocketFactory> socketFactoryClass = (Class<? extends SocketFactory>) Obj.helper.loadClass(socketFactory); Method getDefault = socketFactoryClass.getMethod("getDefault", new Class<?>[]{}); SocketFactory factory = (SocketFactory) getDefault.invoke(null, new Object[]{}); InetSocketAddress endpoint = createInetSocketAddress(host, port); // create unconnected socket socket = factory.createSocket(); // create the socket if (connectTimeout > 0) { if (debug) { System.err.println("Connection: creating socket with " + "a timeout using supplied socket factory"); } // connected socket socket.connect(endpoint, connectTimeout); } else { if (debug) { System.err.println("Connection: creating socket using " + "supplied socket factory"); } // connected socket socket.connect(endpoint); } return socket; } ------------- PR Comment: https://git.openjdk.org/jdk/pull/15294#issuecomment-1681979781