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/common/SubscriberWrapper.java line 347: > 345: > 346: void upstreamWindowUpdate() { > 347: if (pushScheduler.isStopped()) return; A similar question here. It looks to me that the contract with a `SequentialScheduler` is that the runnable task itself (or someone with access to the scheduler), is responsible for invoking the `runOrSchedule` method so that a next invocation of the task happens. When such a scheduler instance is already stopped, then the call to `runOrSchedule` is kind of a noop, with no next execution of the task taking place. In cases like here, where the task detects that the scheduler has already stopped, do you think the tasks should do some cleanup work like clearing up the `outputQ` of `ByteBuffers`? The question really isn't directly related to the PR, but I have been reviewing some unrelated test failures where a large number of HttpClient instances get created, so in that context, I was wondering if there's anything we could do to help reduce any potential memory usage. ------------- PR: https://git.openjdk.java.net/jdk/pull/8562