Hi Daniel, Thank you for your reply and the detailed explanation. I understand the reasoning behind using the client executor only for non-blocking tasks to streamline the exchange itself, and I agree it’s a good approach :)
The main issue, which motivated me to write this email, is that it’s not clear on which executor the returned CompletableFuture executes. In our project, it’s critical to control which thread runs the operations. Currently, the flow looks like this: 1. The method is called from app-thread-1 2. The exchange is executed on exchange-1 3. Subsequent actions are executed on the ForkJoin pool While I can configure the first two executors, the HttpClient does not allow changing the third one, and there’s no way to override it. We discovered this while debugging our application: initially, we assumed callbacks would run on the HTTP executor, but later noticed they execute on the async pool, and were surprised there’s no way to change that, unlike in similar cases elsewhere in the platform. Additionally, since the CompletableFuture default executor is reused, on machines with one or two CPU cores it will fallback to ThreadPerTaskExecutor, causing a new system thread to be created for every HTTP request. This could be avoided if we were able to override the default executor, which would also improve performance on lower-end systems. For example, with the current implementation, if we configure a custom exchange executor and issue 100 HTTP requests without any reply handling on a 2-CPU system, we will end up creating 100 system threads that immediately terminate without doing meaningful work. Instead of introducing a new executor field, perhaps we could have an overloaded version of sendAsync that accepts an executor, similar to CompletableFuture’s API: <U> CompletableFuture<U> thenApplyAsync(Function<? super T, ? extends U> fn, Executor executor) This would give developers clear control over where callbacks run, without complicating the client configuration, and would open the door for optimizations in performance-critical environments. It would not address the two other places where ASYNC_POOL is used (exception handling), but even partial control here would meaningfully improve the client’s flexibility and extensibility. I’d be more than happy to implement the change myself if anyone would like to sponsor it :) Best regards, Michał > Hi Michal, > > >> Would such a change make sense, or is there a strong reason why we > must always fallback to the commonPool? > > > The HttpClient executor is used by the HttpClient for its > own purpose, which is to execute small (usually non-blocking) > asynchronous tasks. > In my todo list I have a task to add an `@implNote` or `@apiNote` >to better explain how the HttpClient uses the executor, and why > tasks submitted to the executor must always be executed. > > The idea was that no blocking code would run in that executor, > so it should be possible to configure the client with an > executor using a very small number of threads (or even an > executor that run tasks inline). > > In practice custom BodyPublishers/BodyHandlers/BodySubscribers > may be invoked while running in the executor, so it's hard to > guarantee that no blocking code would be invoked, but these > should adhere to the reactive stream specification, and thus, if > they adhere to the spec, do nothing blocking. > > Custom dependent actions that are added by the caller to > a CompletableFuture returned by sendAsync() have however no such > requirements, and nothing is there to prevent them from > doing blocking operations. Blocking in the HttpClient executor > would be bad, as it may lead to thread starvation and prevent > requests/responses from eventually completing. This is why we > forcefully ensure that such dependent actions are run in the > Fork Join Pool instead. > > That said, with VirtualThreads being now available in the > platform - I have been wondering whether we should just use > VirtualThreads internally. There are still a few limitations > with VirtualThreads pertaining to class loading and static > initializers, so we haven't switched to that yet, but may > do so in the future. If we eventually do, then maybe the > HttpClient executor could be repurposed to execute (only) > dependent actions. > I am not sure we would like to configure the HttpClient with > yet another executor however. > How big an issue is this? > best regards, > -- daniel > This belongs to the net-dev mailing list, which I CC'ed. > > On Wed, Oct 1, 2025 at 10:56 AM Michał G. [email protected] wrote: > > > Hi all, > > > > I recently ran into an issue with HttpClientImpl where I wanted to handle > > the reply right after calling sendAsync. What surprised me is that the > > returned CompletableFuture already runs on the commonPool, instead of using > > the executor I provided to the HttpClient. > > > > Looking into the implementation, I noticed this piece of code: > > > > // makes sure that any dependent actions happen in the CF default > > // executor. This is only needed for sendAsync(...), when > > // exchangeExecutor is non-null. > > if (exchangeExecutor != null) { > > res = res.whenCompleteAsync((r, t) -> { /* do nothing */}, ASYNC_POOL); > > } > > > > I understand that this exchangeExecutor is meant to cover the > > request/response exchange itself, not necessarily the CompletableFuture > > chaining. But the fact that we always force the returned future back onto > > the commonPool, without any way to change this, feels quite limiting. > > > > In environments where the commonPool is already heavily loaded, this can > > easily introduce performance issues or unpredictable behavior. And since > > > > private static final Executor ASYNC_POOL = new > > CompletableFuture<Void>().defaultExecutor(); > > > > is fixed and cannot be replaced, users don’t have any way around it. For > > comparison, the default executor for CompletableFuture.supplyAsync or for > > parallelStream() is also the commonPool, but in those cases it’s easy to > > override it with a custom executor. It would be nice if HttpClientImpl > > offered the same flexibility. > > > > This is why I’d like to propose a change: when creating an HttpClientImpl, > > it should be possible to specify not only the exchange executor, but also > > the executor used for the returned CompletableFuture > > > > This would be backwards compatible (just an additional optional builder > > parameter), and it could bring several benefits: > > > > reduced context switching for clients that care about which thread executes > > response handling, > > > > more predictable performance in environments where commonPool is shared > > with other workloads, > > > > easier integration with frameworks that already manage their own executors, > > > > clearer control and observability over where callbacks are executed. > > > > Would such a change make sense, or is there a strong reason why we must > > always fallback to the commonPool?
