RFR: 8267140: Support closing the HttpClient by making it auto-closable [v11]
Daniel Fuchs
dfuchs at openjdk.org
Thu Mar 23 11:19:48 UTC 2023
On Thu, 23 Mar 2023 09:31:15 GMT, Daniel Fuchs <dfuchs at openjdk.org> wrote:
>> test/jdk/java/net/httpclient/offline/FixedResponseHttpClient.java line 302:
>>
>>> 300: long timeout = duration.compareTo(max) > 0 ? Long.MAX_VALUE : duration.toMillis();
>>> 301: try {
>>> 302: all.exceptionally((t) -> null).get(timeout, TimeUnit.MILLISECONDS);
>>
>> This implementation of `awaitTermination` only waits for the current (snapshot) of requests that are currently in progress. If a new request happens to be initiated (and is in-progress) after this snapshot of `responses.toArray()` was taken, then this implementation of `awaitTermination` can possibly return an incorrect `true` (indicating the client has terminated) while there currently might be some requests in progress. This `FixedResponseHttpClient` is just a test specific implementation which appears to be used in one single test class (`OfflineTesting`) and as far as I can see, that test won't be impacted with the implementation of `awaitTermination()`, so I think this implementation might be OK for now.
>
> Good point.
OK - this ended up to be a bit more complicated than I thought. What I had missed is that the `FixedResponseHttpClient` actually wraps another (real) `HttpClient`, so close etc... need to close both. I have now fixed this and updated the offline test to close its clients too.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13019#discussion_r1146031449
More information about the net-dev
mailing list