httpClientContext should not be reused between non-consecutive requests

[MosheElisha]

Hi,

Currently the HttpClientContext is stored in HttpClientRestClient#httpClientContext and reused in the various requests.

According to the org.apache.http.protocol.HttpContext docs can be reused but when there are several consecutive messages (for example, redirects) and not when using concurrently in multiple threads.

This issue was raised after I tried to get OpenTracing TracingHttpClientBuilder to work but noticed strange behavior.

The OpenTracing mechanism uses the context to temporarily store the OpenTracing span for cases like redirects.

Using the same context aggregated all the requests under the same trace (which shouldn't have happened).

I suggest to move the logic that creates the HttpClientContext into a method and call it on every submitXxxRequest so a new context will be created for every request.

[Crim]

Hey!

The HttpComponents documents seem to indicate that the Context instance should be re-used for requests relating to the same logical session. I need to do a bit more research on this topic.

Multiple request sequences that represent a logically related session should be executed with the same
HttpContext instance to ensure automatic propagation of conversation context and state information
between requests

[MosheElisha]

Thanks, @Crim . Please check. From what I understand "logically related" are cases like redirects or retries of the same request in the same thread.
Please also see the concurrency warning:

IMPORTANT: Please note HTTP context implementation, even when thread safe, may not be used concurrently by multiple threads, as the context may contain thread unsafe attributes.

[MosheElisha]

@Crim please also see HttpClient: Can i safely reuse HttpContext for different domains by multiple threads?

[MosheElisha]

@Crim can you please do the needed research?

We need a fix ASAP. The current situation with the shared context is:

1. There can be weird behavior when sending requests (like in my case with OpenTracing)
2. It prevents the client from being used concurrently by multiple threads.

Creating a new context on every request is not an expensive operation (especially when we prepare authCache and credsProvider only once). The Apache HttpClient itself creates a new context every time if the user does not pass one.

[Crim]

I'll try to dig into this today or tomorrow and get a better understanding.

That said, the assumption of this library up until this point has been that nothing is thread safe. I'd recommend creating new instances of the client per thread.

[MosheElisha]

Thanks, @Crim !

If you prefer this library will not be thread-safe this is obviously your call but I need a thread-safe library. We have a lot of requests to Kafka Connect and an HttpClient is relatively expensive to create.