Potential bug in HttpClientImpl: Thread pool rejection causes permanent client failure

[David Nadeau]

I do want to follow up on the expected behavior of HttpClient in this case.

If this kind of catastrophic failure within the HttpClient when using
ThreadPools with bounded queues is falling within the expected behaviors of
the HttpClient, then I believe it should be made explicitly clear on the
HttpClient.Builder.executor method javadoc, that ThreadPools with bounded
queues are not supported, and can lead to unrecoverable client failure if
used.

On Wed, Jun 18, 2025 at 11:19 AM David Nadeau <dnadeau at etsy.com> wrote:

> Hi Daniel,
>
> Thank you for this reply.
> In our case, the underlying thread pool doesn't typically become
> overloaded, but when it does, we’re forced to manually restart applications
> because the HttpClient becomes permanently stalled.
>
> Why is it that tasks need to be executed? Dropping work can be an
> acceptable means of backpressure for a threadpool. Are there some tasks
> that the HttpClient submits to this delegate threadpool which are essential
> for the client to continue behaving correctly? And dropping such a task
> could cause the client to continue working, but in a potentially broken way?
> If this is the case, it may be risky to apply my fix of switching to a
> DiscardPolicy. As this would allow the threadpool to silently drop work
> while keeping the http client open. I wonder if it would be safer to use
> your FJP idea to write a RejectedExceptionHandler that falls back to
> submitting rejected tasks there.
>
>
> On Wed, Jun 18, 2025 at 10:42 AM Daniel Fuchs <daniel.fuchs at oracle.com>
> wrote:
>
>> Hi David,
>>
>> As a general rule, the client doesn't expect tasks to
>> be rejected by the executor. Tasks need to be executed,
>> one way or another, for the client to work as expected.
>> Executing in the current thread is often not an option
>> as it may lead to deadlocks.
>> So if we get a RejectedTaskExection, we shutdown the
>> client, as there's not much else we can do.
>>
>> We have some contengency measure for shutdown in case of
>> RejectedTaskException that will temporarily execute rejected
>> tasks in the FJP instead when the executor rejects a task,
>> but this is only a best effort whose only goal is to make
>> it possible to complete the shutdown, and wakeup any calling
>> code that may be waiting on send or completable futures returned
>> by sendAsync.
>>
>> best regards,
>>
>> -- daniel
>>
>> On 18/06/2025 17:24, David Nadeau wrote:
>> > When overriding the HttpClient executor with a custom
>> ThreadPoolExecutor
>> > that uses the default RejectedExecutionHandler.AbortPolicy, the Java
>> > HTTP client (java.net.http.HttpClient) becomes permanently unusable if
>> > the delegate executor rejects a task.
>> >
>> > This creates a situation where transient thread pool saturation results
>> > in a non-recoverable client failure.
>> >
>> > Reproduction of the issue:
>> > 1. The task rejection triggers a call to the error handler
>> (onSubmitFailure)
>> > 2. This calls selmgr.abort(failure) with the rejection exception
>> > 3. SelectorManager.abort() sets this.closed = true permanently
>> > 4. All subsequent HTTP operations fail with "IOException: selector
>> > manager closed"
>> >
>> > The client does not recover from this state.
>> >
>> > I was able to avoid this by using the DiscardPolicy instead of the
>> > AbortPolicy. However, this behavior was quite a surprise to debug. Is
>> > this behavior intentional, or does it make sense for the HttpClient to
>> > treat task rejection as a recoverable error?
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/net-dev/attachments/20250619/0a121532/attachment-0001.htm>