8267140: Support closing the HttpClient by making it auto-closable

[dfuch]

Please find here an RFE that makes the java.net.HttpClient auto-closeable.

The API has been modeled on ExecutorService.

HttpClient::close() is a graceful shutdown and will wait until all operations are terminated before returning.
If a request is in progress, and the caller doesn't pull the corresponding data (for instance, the request was sent with a BodyHandler.ofInputStream(), but the caller stopped reading the input stream) then close() may never return.

Therefore, additional methods similar to those present in ExecutorService are also proposed. In summary:

• shutdown(): initiate a graceful shutdown, but doesn't wait for termination.
• shutdownNow(): initiate an immediate shutdown, attempting to cancel all operations in progress. Doesn't wait for termination.
• awaitTermination(Duration): await for termination within the given delay
• isTerminated() tells whether the client is terminated

New tests have been added to test the proposed behaviors.

HttpClient tests (new and old) are still stable.

---

Progress

• Change must be properly reviewed (1 review required, with at least 1 Reviewer)
• Change must not contain extraneous whitespace
• Commit message must refer to an issue
• Change requires CSR request JDK-8304165 to be approved

Issues

• JDK-8267140: Support closing the HttpClient by making it auto-closable
• JDK-8304165: Support closing the HttpClient by making it auto-closable (CSR)

Reviewers

• Jaikiran Pai (@jaikiran - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/13019/head:pull/13019
$ git checkout pull/13019

Update a local copy of the PR:
$ git checkout pull/13019
$ git pull https://git.openjdk.org/jdk.git pull/13019/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 13019

View PR using the GUI difftool:
$ git pr show -t 13019

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/13019.diff

Webrev

Link to Webrev Comment

[dfuch]

/csr needed

[bridgekeeper]

👋 Welcome back dfuchs! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

[openjdk]

@dfuch has indicated that a compatibility and specification (CSR) request is needed for this pull request.

@dfuch please create a CSR request for issue JDK-8267140 with the correct fix version. This pull request cannot be integrated until the CSR request is approved.

[openjdk]

@dfuch The following label will be automatically applied to this pull request:

• net

When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command.

[mlbridge]

Webrevs

• 16: Full - Incremental (4ebfca14)
• 15: Full - Incremental (6a9523a8)
• 14: Full - Incremental (35c09a02)
• 13: Full - Incremental (e6abd08b)
• 12: Full - Incremental (01142a2d)
• 11: Full - Incremental (6226acff)
• 10: Full - Incremental (2fca2991)
• 09: Full - Incremental (dd4f8f79)
• 08: Full - Incremental (7153f1d5)
• 07: Full - Incremental (387ef77a)
• 06: Full - Incremental (1e0d5191)
• 05: Full - Incremental (4a8e5f88)
• 04: Full - Incremental (43131157)
• 03: Full - Incremental (631dd3ea)
• 02: Full - Incremental (31b5e3fc)
• 01: Full - Incremental (e9aec31e)
• 00: Full (f34c4f14)

[dfuch]

Here is the CSR draft: https://bugs.openjdk.org/browse/JDK-8304165

[dfuch]

Thanks Andrey @turbanoff for the suggestions. I have applied them.

[jaikiran]

The latest updated wording to the specification in HttpClient looks good to me. It adds more clarity on what kind of work might happen (internally in the background) when the client is closed. The implementation changes too look good to me. I've added some test specific comments inline.

Overall, the changes in this PR look good to me and I believe the addition of these new APIs will allow applications to control the lifecycle and resources used by the HttpClient in a much more deterministic way.

[jaikiran]

There a couple of trivial typos in comments, which I have noted inline. The OfflineTesting file would need a copyright year update.

Other than that, the latest changes in 6226acf all look good to me.

[dfuch]

@AlanBateman suggested the following changes, with which I have updated the CSR and this PR:

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 now links to the @implNote

In each of the new methods:

1. The link to the ##closing paragraph (non-normative) is removed from the @implSpec (normative).
2. An @see ##closing Implementation Note on closing the HttpClient is added instead, at the end of the method API documentation.

[openjdk]

@dfuch This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8267140: Support closing the HttpClient by making it auto-closable

Reviewed-by: jpai

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been no new commits pushed to the master branch. If another commit should be pushed before you perform the /integrate command, your PR will be automatically rebased. If you prefer to avoid any potential automatic rebasing, please check the documentation for the /integrate command for further details.

➡️ To integrate this PR with the above commit message to the master branch, type /integrate in a new comment.

[jaikiran]

Hello Daniel, the latest documentation changes all look fine to me.

As for:

2. An `@see ##closing Implementation Note on closing the HttpClient` is added instead, at the end of the method API documentation.

I haven't previously used this construct in a @see, so as long as the generated javadoc renders fine, this looks good. Does it render a <a href=...>Implementation Note on closing the HttpClient</a>?

[dfuch]

/integrate

[dfuch]

I haven't previously used this construct in a @see, so as long as the generated javadoc renders fine, this looks good. Does it render a Implementation Note on closing the HttpClient?

yes - I have generated the api doc and it looks fine

[openjdk]

Going to push as commit 6580c4e.

[openjdk]

@dfuch Pushed as commit 6580c4e.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.