RFR: 8292381: java/net/httpclient/SpecialHeadersTest.java fails with "ERROR: Shutting down connection: HTTP/2 client stopped"
Daniel Fuchs
dfuchs at openjdk.org
Thu Aug 18 09:08:30 UTC 2022
On Thu, 18 Aug 2022 06:52:41 GMT, Jaikiran Pai <jpai at openjdk.org> wrote:
>> Please find here a change that improves SpecialHeadersTest. This test creates a large amount of ephemeral clients and has been observed running out of heap space in our CI once. This change updates the test to wait for the previous HttpClient to be eligible for garbage collection before it creates a new one. It also verifies that no outstanding operation are still running on the client by the time the client is released.
>
> test/jdk/java/net/httpclient/SpecialHeadersTest.java line 195:
>
>> 193: out.printf(now() + "Task %s failed: %s%n", id, t);
>> 194: err.printf(now() + "Task %s failed: %s%n", id, t);
>> 195: FAILURES.putIfAbsent("Task " + id, t);
>
> Hello Daniel, is the `putIfAbsent` intentional here? I wouldn't expect the task id to repeat here (or would there be that many tasks that the `long` id value overflows and repeats?).
It's copy-pasted from other tests that use the same technique. Mainly we want to make sure we get the first error. I agree that `putIfAbsent` might not been necessary but it doesn't hurt either: it makes it clear that the value associated with the key will not be overridden.
> test/jdk/java/net/httpclient/SpecialHeadersTest.java line 284:
>
>> 282: // will be an upgrade
>> 283: if (shared != null) {
>> 284: TRACKER.track(shared);
>
> Is it intentional that we track a shared client when it is being reset, instead of tracking it when we create it (a few lines later)? This is unlike a non-shared instance which we track when we create one.
Yes - because the tracker will complain if any of the tracked client is still alive when check() is called. So the idea is to start tracking the shared client when we're no longer using it.
-------------
PR: https://git.openjdk.org/jdk/pull/9908
More information about the net-dev
mailing list