HttpClient has no explicit way of releasing threads

[Rafael Winterhalter]

Thanks,

I created a shared client for now which solves my issue. I only stumbled
upon this as a build server increased its use of resources and the change
was that somebody had removed a third-party (closeable) HTTP client to
replace it with the JDK client. Unit tests often aim to be decoupled,
therefore I assume this is an easy mistake to make. To be fair, while it
might be inefficient to recreate the client for each test class, the value
of a decoupled test suite might be higher, so I understand why the
developer went for this solution.

I'd appreciate a method for explicit disposal and possibly a clarification
in the javadoc of HttpClient how resources are held. I had to dig through
the sources of the client to understand what happened.

Thanks, Rafael

Daniel Fuchs <daniel.fuchs at oracle.com> schrieb am Di., 10. Mai 2022, 13:37:

> Hi Rafael,
>
> On 09/05/2022 22:43, Rafael Winterhalter wrote:
> > Hello,
> >
> > looking at thread dumps, I realized that the HttpClient implementation
> does
> > not offer an explicit way to release its threads. Currently, the client:
> >
> > 1. maintains a cached thread pool with a retention size of 60 seconds. If
> > many such clients are created for short lived application, these threads
> > pile up.
> > 2. has a selector thread that only shuts down if the outer "facade"
> > reference is collected via a weak reference. If an application is not
> > running GC, this can take a while.
> >
> > This is not a big problem but I have seen peaks with suddenly many, many
> > threads (in test code) where many HttpClients were created for single use
> > and I was wondering if it was ever considered to add a method for
> disposing
> > the threads explicitly?
>
> I would consider it bad practice to create an HttpClient instance for
> single usage; Ideally a client should be reused - provided that
> the security context is the same. Creating an HttpClient involves
> creating a thread pool, a selector, a selector manager thread,
> potentially initializing an SSL context etc...
>
> WRT to adding a method for disposing the HttpClient explicitly, then
> yes - that's something that we could consider for a major
> release. It would need to be carefully specified - especially WRT what
> would be the effect of calling this method if some operations are still
> in progress. Asynchronously closing objects that are still in use is
> a notoriously thorny subject.
> We might need something equivalent to what is defined for executor
> services - that is - one variant that waits for all ongoing operations
> to terminate before closing, and one that abruptly aborts any
> on-going operation.
>
> > Alternatively, it might be an option to add a method like
> > HttpClient.shared() which would return a singleton HttpClient (created on
> > the first call, collected if no reference is kept anymore but reused in
> the
> > meantime) to address such scenarios. I can of course add a singleton in
> my
> > test project but I find this a bit awkward as it is something to remember
> > and to repeat in all applications we maintain. Therefore, it might be
> > convenient to add such methods for tests that usually aim to be
> decoupled.
>
> An HttpClient is a kind of capability object so I don't think we want
> to have a shared client in the Java API. That's something that
> an application can easily implement at the application level if it
> makes sense for the application.
>
> A possibility to work around the thread peak issue is also for an
> application to configure its own thread executor on the HttpClient.
> If that makes sense for the application and if it is safe to do
> so the executor can also be shared between several client.
> It is then the responsibility of the application
> to shutdown the executor when the clients are no longer in use.
>
> >
> > Thanks for your consideration,
> > best regards, Rafael
>
> best regards,
>
> -- daniel
>