JDK-8304165 : Support closing the HttpClient by making it auto-closable
  • Type: CSR
  • Component: core-libs
  • Sub-Component: java.net
  • Priority: P4
  • Status: Closed
  • Resolution: Approved
  • Fix Versions: 21
  • Submitted: 2023-03-14
  • Updated: 2023-04-06
  • Resolved: 2023-04-06
Related Reports
CSR :  
Relates :  
Description
Summary
-------

Make the `java.net.http.HttpClient` implement `AutoCloseable` in order to make it possible to reclaim its resources early (selector, idle connections sitting in pools, etc...) rather than having to wait for GC to garbage collect it.

Problem
-------

The `java.net.http.HttpClient` does not expose any `close()` or `shutdown()` method. Users of the API have to rely on GC garbaging the HttpClient facade to trigger the exit of the client's selector manager thread and the reclamation of associated resources, such as closing idle connections sitting in the client's pool. 

Solution
--------

Make `java.net.http.HttpClient` implement `AutoCloseable`. 
The `close()` method would implement a graceful shutdown, waiting for already submitted requests to complete. 

However, waiting for requests to complete gracefully may depend on callers to actually read the data sent by the server, so close() may never complete. For instance if a caller calls `send` or `sendAsync` passing `BodyHandlers.ofInputStream()`, stops reading the input stream before `EOF` is reached, and never closes the stream.
This is not dissimilar with the problem of closing an `ExecutorService`, if a task never terminates. This CSR thus proposes to add additional methods to the `java.net.http.HttpClient`, modeled on what was done for `ExecutorService`. In summary, the following methods are proposed:

 - `void close()`: closes the client gracefully, waiting for submitted requests to complete.
 - `void shutdown()`: initiate a graceful shutdown, then return immediately without waiting for the client to terminate.
 - `void shutdownNow()`: initiate an immediate shutdown, trying to interrupt active operations, and return immediately without waiting for the client to terminate.
 - `boolean awaitTermination(Duration duration)`, wait for the client to terminate, within the given duration; returns true if the client is terminated, false otherwise.
 - `boolean isTerminated()`: returns true if the client is terminated.



Specification
-------------

In `java.net.http/java.net.http.HttpClient`:

`HttpClient` now implements `AutoCloseable`:

    --- a/src/java.net.http/share/classes/java/net/http/HttpClient.java
    +++ b/src/java.net.http/share/classes/java/net/http/HttpClient.java
    @@ -59,11 +63,15 @@
      * protocol version ( HTTP/1.1 or HTTP/2 ), whether to follow redirects, a
      * proxy, an authenticator, etc. Once built, an {@code HttpClient} is immutable,
      * and can be used to send multiple requests.
      *
      * <p> An {@code HttpClient} provides configuration information, and resource
    - * sharing, for all requests sent through it.
    + * sharing, for all requests sent through it. An {@code HttpClient} instance
    + * typically manages its own pools of connections, which it may then reuse
    + * as and when necessary. Connection pools are  typically not shared between
    + * {@code HttpClient} instances. Creating a new client for each operation,
    + * though possible, will usually prevent reusing such connections.
      *
      * <p> A {@link BodyHandler BodyHandler} must be supplied for each {@link
      * HttpRequest} sent. The {@code BodyHandler} determines how to handle the
      * response body, if any. Once an {@link HttpResponse} is received, the
      * headers, response code, and body (typically) are available. Whether the
    @@ -117,11 +125,33 @@
      * been configured. The form of the {@code URLPermission} required to access a
      * proxy has a {@code method} parameter of {@code "CONNECT"} (for all kinds of
      * proxying) and a {@code URL} string of the form {@code "socket://host:port"}
      * where host and port specify the proxy's address.
      *
    - * @implNote If an explicit {@linkplain HttpClient.Builder#executor(Executor)
    + * @apiNote
    + * Resources allocated by the {@code HttpClient} may be
    + * reclaimed early by {@linkplain #close() closing} the client.
    + *
    + * @implNote
    + *  <p id="closing">
    + *  The JDK built-in implementation of the {@code HttpClient} overrides
    + * {@link #close()}, {@link #shutdown()}, {@link #shutdownNow()},
    + * {@link #awaitTermination(Duration)}, and {@link #isTerminated()} to
    + * provide a best effort implementation. Failing to close, cancel, or
    + * read returned streams to exhaustion, such as streams provided when using
    + * {@link BodyHandlers#ofInputStream()}, {@link BodyHandlers#ofLines()}, or
    + * {@link BodyHandlers#ofPublisher()}, may prevent requests submitted
    + * before an {@linkplain #shutdown() orderly shutdown}
    + * to run to completion. Likewise, failing to
    + * {@linkplain Subscription#request(long) request data} or {@linkplain
    + * Subscription#cancel() cancel subscriptions} from a custom {@linkplain
    + * java.net.http.HttpResponse.BodySubscriber BodySubscriber} may stop
    + * delivery of data and {@linkplain #awaitTermination(Duration) stall an
    + * orderly shutdown}.
    + *
    + * <p>
    + * If an explicit {@linkplain HttpClient.Builder#executor(Executor)
      * executor} has not been set for an {@code HttpClient}, and a security manager
      * has been installed, then the default executor will execute asynchronous and
      * dependent tasks in a context that is granted no permissions. Custom
      * {@linkplain HttpRequest.BodyPublisher request body publishers}, {@linkplain
      * HttpResponse.BodyHandler response body handlers}, {@linkplain
    @@ -130,11 +160,11 @@
      * privileges, should do so within an appropriate {@linkplain
      * AccessController#doPrivileged(PrivilegedAction) privileged context}.
      *
      * @since 11
      */
    -public abstract class HttpClient {
    +public abstract class HttpClient implements AutoCloseable {

API documentation of send/sendAsync now state that IOException will be thrown if the client is shut down:

HttpClient::send

          * @return the response
    -     * @throws IOException if an I/O error occurs when sending or receiving
    +     * @throws IOException if an I/O error occurs when sending or receiving, or
    +     *         the client has {@linkplain ##closing shut down}
          * @throws InterruptedException if the operation is interrupted
          * @throws IllegalArgumentException if the {@code request} argument is not
          *         a request that could have been validly built as specified by {@link

    
HttpClient::sendAsync (3 args method):

          * <p> The returned completable future completes exceptionally with:
          * <ul>
    -     * <li>{@link IOException} - if an I/O error occurs when sending or receiving</li>
    +     * <li>{@link IOException} - if an I/O error occurs when sending or receiving,
    +     *      or the client has {@linkplain ##closing shut down}.</li>
          * <li>{@link SecurityException} - If a security manager has been installed
          *          and it denies {@link java.net.URLPermission access} to the
          *          URL in the given request, or proxy if one is configured.

New methods:

    +
    +    /**
    +     * Initiates an orderly shutdown in which  requests previously
    +     * submitted with {@code send} or {@code sendAsync}
    +     * are run to completion, but no new request will be accepted.
    +     * Running a request to completion may involve running several
    +     * operations in the background, including {@linkplain ##closing
    +     * waiting for responses to be delivered}, which will all have to
    +     * run to completion until the request is considered completed.
    +     *
    +     * Invocation has no additional effect if already shut down.
    +     *
    +     * <p>This method does not wait for previously submitted request
    +     * to complete execution.  Use {@link #awaitTermination(Duration)
    +     * awaitTermination} or {@link #close() close} to do that.
    +     *
    +     * @implSpec
    +     * The default implementation of this method does nothing. Subclasses should
    +     * override this method to implement the appropriate behavior.
    +     *
    +     * @see ##closing Implementation Note on closing the HttpClient
    +     *
    +     * @since 21
    +     */
    +    public void shutdown() { }
    +
    +    /**
    +     * Blocks until all operations have completed execution after a shutdown
    +     * request, or the {@code duration} elapses, or the current thread is
    +     * {@linkplain Thread#interrupt() interrupted}, whichever happens first.
    +     * Operations are any tasks required to run a request previously
    +     * submitted with {@code send} or {@code sendAsync} to completion.
    +     *
    +     * <p> This method does not wait if the duration to wait is less than or
    +     * equal to zero. In this case, the method just tests if the thread has
    +     * terminated.
    +     *
    +     * @implSpec
    +     * The default implementation of this method checks for null arguments, but
    +     * otherwise does nothing and returns true.
    +     * Subclasses should override this method to implement the proper behavior.
    +     *
    +     * @param duration the maximum time to wait
    +     * @return {@code true} if this client terminated and
    +     *         {@code false} if the timeout elapsed before termination
    +     * @throws InterruptedException if interrupted while waiting
    +     *
    +     * @see ##closing Implementation Note on closing the HttpClient
    +     *
    +     * @since 21
    +     */
    +    public boolean awaitTermination(Duration duration) throws InterruptedException {
    +        Objects.requireNonNull(duration);
    +        return true;
    +    }
    +
    +    /**
    +     * Returns {@code true} if all operations have completed following
    +     * a shutdown.
    +     * Operations are any tasks required to run a request previously
    +     * submitted with {@code send} or {@code sendAsync} to completion.
    +     * <p> Note that {@code isTerminated} is never {@code true} unless
    +     * either {@code shutdown} or {@code shutdownNow} was called first.
    +     *
    +     * @implSpec
    +     * The default implementation of this method does nothing and returns false.
    +     * Subclasses should override this method to implement the proper behavior.
    +     *
    +     * @return {@code true} if all tasks have completed following a shutdown
    +     *
    +     * @see ##closing Implementation Note on closing the HttpClient
    +     *
    +     * @since 21
    +     */
    +    public boolean isTerminated() {
    +        return false;
    +    }
    +
    +    /**
    +     * This method attempts to initiate an immediate shutdown.
    +     * An implementation of this method may attempt to
    +     * interrupt operations that are actively running.
    +     * Operations are any tasks required to run a request previously
    +     * submitted with {@code send} or {@code sendAsync} to completion.
    +     * The behavior of actively running operations when interrupted
    +     * is undefined. In particular, there is no guarantee that
    +     * interrupted operations will terminate, or that code waiting
    +     * on these operations will ever be notified.
    +     *
    +     * @implSpec
    +     * The default implementation of this method simply calls {@link #shutdown()}.
    +     * Subclasses should override this method to implement the appropriate
    +     * behavior.
    +     *
    +     * @see ##closing Implementation Note on closing the HttpClient
    +     *
    +     * @since 21
    +     */
    +    public void shutdownNow() {
    +        shutdown();
    +    }
    +
    +    /**
    +     * Initiates an orderly shutdown in which  requests previously
    +     * submitted to {@code send} or {@code sendAsync}
    +     * are run to completion, but no new request will be accepted.
    +     * Running a request to completion may involve running several
    +     * operations in the background, including {@linkplain ##closing
    +     * waiting for responses to be delivered}.
    +     * This method waits until all operations have completed execution
    +     * and the client has terminated.
    +     *
    +     * <p> If interrupted while waiting, this method may attempt to stop all
    +     * operations by calling {@link #shutdownNow()}. It then continues to wait
    +     * until all actively executing operations have completed.
    +     * The interrupt status will be re-asserted before this method returns.
    +     *
    +     * <p> If already terminated, invoking this method has no effect.
    +     *
    +     * @implSpec
    +     * The default implementation invokes {@code shutdown()} and waits for tasks
    +     * to complete execution with {@code awaitTermination}.
    +     *
    +     * @see ##closing Implementation Note on closing the HttpClient
    +     *
    +     * @since 21
    +     */
    +    @Override
    +    public void close() {
    +        [...]
    +    }
    +











Comments
Moving to Approved.
06-04-2023

Alan suggested the following changes, with which I have updated this CSR: In the class level documentation: 1. Only the first sentence of the previously added `@apiNote` remains in the `@apiNote` 2. The bulk of the `@apiNote` text that was added previously is now moved to the `@implNote` paragraph 3. The modified `@apiNote` is moved above the `@implNote` that was previously there 4. The ##closing anchor links to the implNote In each of the new methods: 1. The link to the ##closing paragraph (non-normative) is removed from the impl spec (normative). 2. An @see ##closing Implementation Note on closing the HttpClient is added instead, at the end of the method API documentation. I hope that solves the issue with `@implSpec` previously referring to an `@apiNote`.
05-04-2023

Thanks for looking at this Joe! Changed to: + * @implSpec + * The default implementation of this method check for null arguments, but otherwise + * does nothing and returns true. + * Subclasses should override this method to implement the proper behavior. + * See the API Note on {@linkplain ##closing closing}. + * + * @param duration the maximum time to wait As regard to the `@apiNote` it really describes the behavior of the JDK implementation of the API, and informs the reader about the necessity of closing the closeable objects returned from the client - which in my mind was describing how to use the API properly. Do you really think it should be an `@implSpec`? If so would it apply to any class extending HttpClient (which I was trying to avoid) - or just to the instances returned by newHttpClient/newBuilder (that is - just to another implementation of the `java.net.http` module?
04-04-2023

Moving to Provisional, not Approved. As a code review comment, the implSpec and included implementation of awaitTermination are not consistent: + * @implSpec + * The default implementation of this method does nothing and returns true. + * Subclasses should override this method to implement the proper behavior. + * See the API Note on {@linkplain ##closing closing}. ... + public boolean awaitTermination(Duration duration) throws InterruptedException { + Objects.requireNonNull(duration); + return true; + } Checking for null of the argument is very little, but not quite nothing. Also, should the apiNote of HttpClient by upgraded to implSpec? This should like a property we want to require rather than just being an advisory note.
04-04-2023