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