On Tue, 9 Dec 2025 16:03:15 GMT, Jaikiran Pai <[email protected]> wrote:

>> Can I please get a review of this test-only change which proposes to address 
>> the intermittent failures in `com/sun/jndi/ldap/LdapPoolTimeoutTest.java`?
>> 
>> As noted in https://bugs.openjdk.org/browse/JDK-8287062, this test fails 
>> intermittently. This test was introduced in 
>> https://bugs.openjdk.org/browse/JDK-8277795 with the goal to verify that the 
>> LDAP connection timeout was honoured when concurrent connections were 
>> attempted. The test itself is simple and launches the LDAP connection 
>> attempts concurrently and expects each attempt to fail with an exception. In 
>> order to verify that the failure is for the right reasons, it tries its best 
>> to check the exception type and exception message. If they don't match the 
>> exception messages this test knows of, the exception is propagated to fail 
>> the test.
>> 
>> It so happens that different parts of LDAP connection management code and 
>> some other parts of java.net.Socket code might emit different exception 
>> messages and in some cases the java.net.Socket might emit the right 
>> exception type but without any exception message. Some of the recent 
>> sightings of this test failure have this stacktrace:
>> 
>> 
>> javax.naming.CommunicationException: example.com:1234 [Root exception is 
>> java.net.SocketTimeoutException]
>>      at java.naming/com.sun.jndi.ldap.Connection.<init>(Connection.java:251)
>>      at java.naming/com.sun.jndi.ldap.LdapClient.<init>(LdapClient.java:136)
>>      at 
>> java.naming/com.sun.jndi.ldap.LdapClientFactory.createPooledConnection(LdapClientFactory.java:71)
>>      at 
>> java.naming/com.sun.jndi.ldap.pool.Connections.createConnection(Connections.java:184)
>>      at 
>> java.naming/com.sun.jndi.ldap.pool.Connections.getAvailableConnection(Connections.java:150)
>>      at 
>> java.naming/com.sun.jndi.ldap.pool.Pool.getOrCreatePooledConnection(Pool.java:196)
>>      at 
>> java.naming/com.sun.jndi.ldap.pool.Pool.getPooledConnection(Pool.java:152)
>>      at 
>> java.naming/com.sun.jndi.ldap.LdapPoolManager.getLdapClient(LdapPoolManager.java:339)
>>      ...
>>      at 
>> java.naming/javax.naming.directory.InitialDirContext.<init>(InitialDirContext.java:130)
>>      at 
>> LdapPoolTimeoutTest.lambda$attemptConnect$0(LdapPoolTimeoutTest.java:114)
>>      at LdapTimeoutTest.lambda$assertCompletion$0(LdapTimeoutTest.java:361)
>>      at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:330)
>>      at java.base/java.lang.Thread.run(Thread.java:1516)
>> Caused by: java.net.SocketTimeoutException
>>      at 
>> java.base/java.net.SocksSocketImpl.remainingMillis(SocksSocketImpl.java:93)
>>      at java.base/java.n...
>
> Jaikiran Pai has updated the pull request incrementally with one additional 
> commit since the last revision:
> 
>   fail if InitialDirContext() construction completes normally

Nice clean up of the exception handling code. Looks good to me!

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

Marked as reviewed by aefimov (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/28725#pullrequestreview-3577449822

Reply via email to