Major Description
Details
We're getting a NullPointerException exception in versions >= 4.4.5. The exception occurs ~20 times per 10m requests in DefaultConnectionReuseStrategy when calling request.headerIterator. The change that introduced it was swapping to checking the request for keepAlive instead of just the response [1] in version 4.4.5. From looking at the source in github, this change is still in version 5.0, so the NullPointerException may be in versions > 4.4.9, but that's the latest version I have tested on so far.
From testing the source of the exception is multiple threads accessing the HeaderGroup, with one thread calling setHeader & the other calling iterator(name). The HeaderGroup is backed by a thread-unsafe ArrayList, which is the source of the exception. I have a fix for this, is there somewhere I can submit a pull request?
The fix is to swap to a thread safe CopyOnWriteArrayList. This will have a performance impact, but it should be minor since the list is small (16 indices) and headers don't appear to be modified frequently.
Client details
Here's a simplified version of our client. From comparing it to the examples provided in the docs there isn't anything that appears to be thread-unsafe aside from the shared HttpClientContext, which I've confirmed is not the root cause by making it thread local.
// single threaded
// Isn't necessarily thread safe, I've tried moving it into the "post" method to see if it is the cause, but the exceptions persist.
private final HttpClientContext context = HttpClientContext.adapt(new BasicHttpContext());
final RegistryBuilder<ConnectionSocketFactory> registryBuilder = RegistryBuilder.<ConnectionSocketFactory>create().register("https", sslSocketFactory.get());.
final PoolingHttpClientConnectionManager connectionManager = new PoolingHttpClientConnectionManager(registryBuilder.build())
connectionManager.setMaxTotal(connectionPoolSize);
connectionManager.setDefaultMaxPerRoute(maxConnectionsPerRoute);
connectionManager.setValidateAfterInactivity(duration);
final CloseableHttpClient client = HttpClients.custom()
.setConnectionManager(connectionManager)
.setKeepAliveStrategy((response, context) -> keepAliveDuration.toMillis())
.setUserAgent(userAgent)
.build();
// removing this does not fix the NullPointerException
executorService.scheduleAtFixedRate(
() -> connectionManager.closeIdleConnections(idleConnectionExpiry.toMillis(),
TimeUnit.MILLISECONDS),
Duration.ofSeconds(30).toMillis(),
Duration.ofSeconds(30).toMillis(),
TimeUnit.MILLISECONDS);
// Called from multiple threads
Result post(uri, config, entity, hostName) {
final HttpPost req = new HttpPost(uri);
req.setConfig(config);
req.setEntity(entity);
req.addHeader(HttpHeaders.HOST, hostName);
try (CloseableHttpResponse response = client.execute(req, context))
catch(...)
{ // handle exception }}