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

Reply via email to