> 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

Reply via email to