RFR: 8267140: Support closing the HttpClient by making it auto-closable [v4]

[Daniel Fuchs]

On Mon, 20 Mar 2023 13:02:20 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:

>> Daniel Fuchs has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains 10 additional commits since the last revision:
>> 
>>  - Merge branch 'master' into HttpClient-close-8267140
>>  - Update test/jdk/java/net/httpclient/HttpClientShutdown.java
>>    
>>    Co-authored-by: Andrey Turbanov <turbanoff at gmail.com>
>>  - Update test/jdk/java/net/httpclient/ShutdownNow.java
>>    
>>    Co-authored-by: Andrey Turbanov <turbanoff at gmail.com>
>>  - Update test/jdk/java/net/httpclient/HttpClientShutdown.java
>>    
>>    Co-authored-by: Andrey Turbanov <turbanoff at gmail.com>
>>  - Update test/jdk/java/net/httpclient/AsyncShutdownNow.java
>>    
>>    Co-authored-by: Andrey Turbanov <turbanoff at gmail.com>
>>  - Review feedback
>>  - typo
>>  - Merge branch 'master' into HttpClient-close-8267140
>>  - Throw NPE if duration is null
>>  - 8267140
>
> src/java.net.http/share/classes/java/net/http/HttpClient.java line 773:
> 
>> 771:      *
>> 772:      * @implSpec
>> 773:      * The default implementation of this method does nothing and returns true.
> 
> It's a bit odd that this default implementation would return `true` (meaning the client has terminated) and then the default implemenation of `isTerminated()` method is returning `false`. I think the implementation of this default method should return `false`, both to match the value returned by `isTerminated` as well as because the default implementation would not really know if the client is terminated.

If that where the case, then `close()` would never return when none of these methods are overridden (case of existing subclasses).  `isTerminated` stipulates that it will never return `true` if shutdown has not been called, which is borrowed from `ExecutorService` specification, and maintaining a state in this super class is not really a good idea. The choice was between throwing an exception (that would make close throw too), or return `true`, which would allow close() to trivially return if not implemented. All in all I believe I prefer to have `awaitTermination` returning `true` by default, even though I agree that it's a bit odd. But it keeps things simple.

-------------

PR: https://git.openjdk.org/jdk/pull/13019