> 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 incrementally with two additional commits since the last revision: - move the new test under test/jdk/com/sun/jndi/ldap - Aleksei's review - use public/standard APIs instead of internal com.sun.jndi.ldap.LdapCtx ------------- Changes: - all: https://git.openjdk.org/jdk/pull/25449/files - new: https://git.openjdk.org/jdk/pull/25449/files/ca8c19a8..c5a222ae Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=25449&range=05 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=25449&range=04-05 Stats: 16 lines in 1 file changed: 9 ins; 4 del; 3 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
