> 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 six additional commits since the last revision: - remove format() call in exception message creation - merge latest from master branch - merge latest from master branch - 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/545c41a1..5d6c2fdf Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=25449&range=03 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=25449&range=02-03 Stats: 96717 lines in 2693 files changed: 56785 ins; 27619 del; 12313 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
