On Thu, 5 May 2022 19:03:13 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`)
src/java.net.http/share/classes/jdk/internal/net/http/HttpClientImpl.java line
1039:
> 1037: e.abort(selectorClosedException());
> 1038: } else {
> 1039: selector.wakeup();
Hello Daniel, before this PR, except for the `wakeupSelector()` method on
`SelectorManager`, all other methods which were operating on the `selector`
field were `synchronized` on the `SelectorManager` instance. After this PR,
that continues to be true except for this specific line where the operation on
the `selector` field is outside of a `synchronized` block. Would that be OK?
-------------
PR: https://git.openjdk.java.net/jdk/pull/8562
