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

Good to see the dead code removed from HttpClientImpl (cancel). Question about 
the code copied in from ThreadInfo. Is there any way we could request a change 
to that class to adjust the number of stack frames printed? Okay, I see the 
same issue has been raised already. I agree we should request that enhancement.

-------------

PR: https://git.openjdk.java.net/jdk/pull/8562

Reply via email to