> 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.
Jaikiran Pai has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision: - merge latest from master branch - add test - 8357708: com.sun.jndi.ldap.Connection ignores queued LDAP replies if Connection is subsequently closed ------------- Changes: - all: https://git.openjdk.org/jdk/pull/25449/files - new: https://git.openjdk.org/jdk/pull/25449/files/908e4b60..5a6012e6 Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=25449&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=25449&range=00-01 Stats: 120533 lines in 2273 files changed: 72238 ins; 31047 del; 17248 mod Patch: https://git.openjdk.org/jdk/pull/25449.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/25449/head:pull/25449 PR: https://git.openjdk.org/jdk/pull/25449
