RFR: 8286194: ExecutorShutdown test fails intermittently

[Daniel Fuchs]

On Fri, 6 May 2022 04:49:53 GMT, Jaikiran Pai <jpai at 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?

And this is what (or at least a part of what) was causing the issue. We need to add the event to the `registrations` list within a synchronized block because that's a plain ArrayList whose access is controlled by synchronizing on `this`. However the event should not be invoked within the synchronized and block, and if you look at the calling method (eventUpdated) you will see that this is what we are doing there too (raising the event without synchronizing).

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

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