Search code examples
javaoverridingapache-httpclient-4.xautocloseable

Stop callers from accidentally closing CloseableHttpClient


I am configuring a class to manage an Apache HttpClient that will be used application-wide for the lifetime of the application, like the following:

public class HttpManager {
  private CloseableHttpClient client;
  private HttpClientBuilder builder;

  public HttpClient getClient() {
    // throw if null
    return client;
  }

  /** 
   * Returns the builder configured with default settings, 
   * in case a new client needs to be made with a different config for
   * a specific purpose.
   */
  public HttpClientBuilder getClientBuilder() {
    // throw if null
    return builder;
  }

  // repeat for HttpAsyncClient

  public void startup() {
    // construct builders and clients, read from config
  }

  public void shutdown() {
    client.close();
  }
}

However, returning an HttpClient does not provide CloseableHttpResponses from its execute functions. Returning a CloseableHttpClient allows execute to return the closeable response, but it also exposes close() on the client, and I don't want other modules to accidentally close the client before the rest of the application is done with it. Is there a way to provide a CloseableHttpClient that gives a CloseableHttpResponse but does not allow itself to be closed?

I initially tried to find a way to override or wrap just the close() method of the client, so that users external to HttpManager would get an UnsupportedOperationException. However, I'm a little wary of creating a class like this:

class UncloseableHttpClient extends CloseableHttpClient {
  CloseableHttpClient delegate;
  // basic constructor

  @Override 
  public CloseableHttpResponse doExecute(HttpHost target, HttpRequest request, HttpContext context) throws IOException {
    return delegate.doExecute(target, request, context);
  }

  @Override
  public void close() {
    throw new UnsupportedOperationException("Only HttpManager should close this client!");
  }

  // delegate other methods, including ones that are deprecated
}

Extending CloseableHttpClient looks like it might have side affects I'm not confident in evaluating.

Returning a plain HttpClient doesn't allow for use of the try-with-resources statement for HttpResponse, and I've had trouble with connection leaks from response entities not being consumed properly.

The overall goal of the manager is to centralize proxy logic and thread pooling, as right now it's very common in the application to create and discard clients per-module, per-transaction, or even per-request.

To note: this method of returning a CloseableHttpClient might not be needed if there's a better way to manage connections such as with builder.setConnectionManager(). I want to be able to stop connection leaks on the client itself if possible, and provide the try-with-resources CloseableHttpResponse if not.


Solution

  • After testing a bit more, I can confirm that creating an UncloseableHttpClient class like in my question would work; however, there is a better way.

    Instead, create an HttpClientConnectionManager and maintain that in some singleton location for the life of the application, and when building the HttpClient, provide it as the connection manager and use the builder to mark that the connection manager is shared.

    class HttpClientManager {
      private final PoolingHttpClientConnectionManager connectionManager = new PoolingHttpClientConnectionManager();
    
      public CloseableHttpClient getDefaultClient() {
        return HttpClientBuilder.create()
            .setConnectionManager(connectionManager)
            .setConnectionManagerShared(true)
            // set proxy router and credential provider as needed
            .build();
      }
    
      public void shutdown() throws IOException {
        connectionManager.shutdown();
      }
    }
    

    Callers of getDefaultClient() can then call close() on their client instances whenever they like, without affecting behavior of other components within the application.