On Sat, 7 May 2022 11:46:37 GMT, Daniel Fuchs <dfu...@openjdk.org> wrote:
>> Hi, please find here a patch that solves a rare intermittent test failure >> observed in the test `java/net/httpclient/ExecutorShutdown.java` >> >> A race condition coupled with some too eager synchronization was causing a >> deadlock between an Http2Connection close, a thread trying to shutdown the >> HttpClient due to a RejectedTaskException, and the SelectorManager thread >> trying to exit. >> >> The fix comprises several cleanup - in particular: >> >> - `Http2Connection`: no need to try to send a `GOAWAY` frame if the >> underlying TCP connection is already closed >> - `SSLFlowDelegate`/`SubscriberWrapper`: no need to trigger code that will >> request more data from upstream if the sequential scheduler that is supposed >> to handle that data once it arrives is already closed >> - `Http1Exchange`/`Http1Request`: proper cancellation of subscription if an >> exception is raised before `onSubscribe()` has been called >> - `HttpClientImpl`: avoid calling callbacks from within synchronized blocks >> when not necessary >> - `ReferenceTracker`: better thread dumps in case where the selector is >> still alive at the end of the test (remove the limit that limited the stack >> traces to 8 element max by no longer relying on `ThreadInfo::toString`) > > Daniel Fuchs 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 branch 'master' into ExecutorShutdown-intermittent-8286194 > - Added a comment to ReferenceTracker.java as suggested in the review > - 8286194: ExecutorShutdown test fails intermittently Change looks fine to me. We can revisit the ThreadInfo code later. ------------- Marked as reviewed by michaelm (Reviewer). PR: https://git.openjdk.java.net/jdk/pull/8562
