On Mon, 21 Aug 2023 22:10:06 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:
> 
>   use original logic to create socket to avoid the compatibility issue

The latest version looks fine to me:
New `createConnectionSocket` methods make code more readable while 
maintaining the same codeflow in respect of connection timeout and
socket factory method calls.
Existing LDAP tests show no problems with the proposed fix.
 
Few minor formatting suggestions:
 
Remove unused imports in the test:

 import java.io.File;
-import java.io.FileInputStream;
-import java.io.FileOutputStream;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
@@ -36,7 +34,6 @@ import javax.naming.ldap.InitialLdapContext;
 import javax.naming.ldap.LdapContext;
 import javax.naming.Context;
 import java.net.SocketException;
-import java.security.KeyStore;


Remove extra space:

-        public CustomSocket () {
+        public CustomSocket() {


Remove redundant default value:

-        private boolean isForceToSleep = false;
+        private boolean isForceToSleep;

-------------

PR Review: https://git.openjdk.org/jdk/pull/15294#pullrequestreview-1589632056

Reply via email to