On Mon, 26 May 2025 12:28:16 GMT, Jaikiran Pai <[email protected]> wrote:
> Can I please get a review of this change which proposes to address the issue > noted in https://bugs.openjdk.org/browse/JDK-8357708? > > As noted in the issue, the current code in > `com.sun.jndi.ldap.Connection.readReply()` is susceptible to throwing a > `ServiceUnavailableException` even when the LDAP replies have already been > received and queued for processing. The JBS issue has additional details > about how that can happen. > > The commit in this PR simplifies the code in `com.sun.jndi.ldap.LdapRequest` > to make sure it always gives out the replies that have been queued when the > `LdapRequest.getReplyBer()` gets invoked. One of those queued values could be > markers for a cancelled or closed request. In that case, the `getReplyBer()`, > like previously, continues to throw the right exception. With this change, > the call to `replies.take()` or `replies.poll()` (with an infinite timeout) > is no longer expected to hang forever, if the `Connection` is closed (or the > request cancelled). This then allows us to remove the connection closure > (`sock == null`) check in `Connection.readReply()`. > > A new jtreg test has been introduced to reproduce this issue and verify the > fix. The test reproduces this issue consistently when the source fix isn't > present. With the fix present, even after several thousand runs of this test, > the issue no longer reproduces. > > tier1, tier2 and tier3 tests continue to pass with this change. I've marked > the fix version of this issue for 26 and I don't plan to push this for 25. This pull request has now been integrated. Changeset: e2eaa2ec Author: Jaikiran Pai <[email protected]> URL: https://git.openjdk.org/jdk/commit/e2eaa2ecf39f1746481bfdd603e84167c886545b Stats: 541 lines in 3 files changed: 446 ins; 60 del; 35 mod 8357708: com.sun.jndi.ldap.Connection ignores queued LDAP replies if Connection is subsequently closed Co-authored-by: Aleksei Efimov <[email protected]> Reviewed-by: aefimov, dfuchs ------------- PR: https://git.openjdk.org/jdk/pull/25449
