On Wed, 17 Jun 2026 13:41:52 GMT, Jaikiran Pai <[email protected]> wrote:
>> Ashay Rane has updated the pull request incrementally with one additional
>> commit since the last revision:
>>
>> Handle the case when `closureReason == null`
>>
>> Also, since `closureReason` is shared, we first read it into a local
>> variable (`priorFailure`) before checking for null.
>
> src/java.naming/share/classes/com/sun/jndi/ldap/Connection.java line 1177:
>
>> 1175: CommunicationException ce = new
>> CommunicationException();
>> 1176: ce.setRootCause(closureReason);
>> 1177: ce.addSuppressed(ex);
>
> Hello Ashay, I think this will need a slightly different change. The
> `closureReason` may be null, so it might be better to do something like (this
> untested change):
>
>
> diff --git a/src/java.naming/share/classes/com/sun/jndi/ldap/Connection.java
> b/src/java.naming/share/classes/com/sun/jndi/ldap/Connection.java
> --- a/src/java.naming/share/classes/com/sun/jndi/ldap/Connection.java
> +++ b/src/java.naming/share/classes/com/sun/jndi/ldap/Connection.java
> @@ -1173,8 +1173,14 @@ public void handshakeCompleted(HandshakeCompletedEvent
> event) {
> tlsHandshakeCompleted.complete(tlsServerCert);
> } catch (SSLPeerUnverifiedException ex) {
> CommunicationException ce = new CommunicationException();
> - ce.setRootCause(closureReason);
> - tlsHandshakeCompleted.completeExceptionally(ex);
> + IOException priorFailure = closureReason;
> + if (priorFailure != null) {
> + ce.setRootCause(priorFailure);
> + ce.addSuppressed(ex);
> + } else {
> + ce.setRootCause(ex);
> + }
> + tlsHandshakeCompleted.completeExceptionally(ce);
> }
> }
> }
>
> The tests for this area reside in `tier2`. So please also run tier2 locally
> to make sure nothing fails due to this change.
Thanks for the fix to handle the case when `closureReason` could be NULL. I've
updated the patch and ran the test/jdk:tier2 tests and they all pass.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/31545#discussion_r3454176990
