RFR: JDK-8311792: java/net/httpclient/ResponsePublisher.java fails intermittently with AssertionError: Found some outstanding operations

[Jaikiran Pai]

On Wed, 16 Aug 2023 12:23:27 GMT, Darragh Clarke <dclarke at openjdk.org> wrote:

> Currently `ResponsePublisher` occasionally fails due to unreleased resources.
> Updated test based on [AbstractThrowingPushPromises](https://github.com/openjdk/jdk/blob/b80001de0c0aeedeb412430660a4727fc26be98b/test/jdk/java/net/httpclient/AbstractThrowingPushPromises.java#L343) to ensure that non-shared clients get closed before further iterations run, this should limit the max number of clients that remain alive during the test.
> 
> I ran tiers 1-3 and everything is passing

test/jdk/java/net/httpclient/ResponsePublisher.java line 498:

> 496:     // we use the ReferenceTracker API rather than HttpClient::close here,
> 497:     // because these tests inject faults by throwing inside callbacks, which
> 498:     // is more likely to get HttpClient::close wedged until jtreg times out.

Is this comment and the decision to not use `HttpClient.close()` in this test relevant to this test case? In `AbstractThrowingPublishers`, which is where this comment initially resided like you note in the PR description, I think it was relevant. I haven't thoroughly looked at the tests in this `ResponsePublisher` test, so unsure if we can't/shouldn't use `HttpClient.close()` in these test methods.

test/jdk/java/net/httpclient/ResponsePublisher.java line 506:

> 504:         System.out.println(now() + "waiting for client to shutdown: " + tracker.getName());
> 505:         System.err.println(now() + "waiting for client to shutdown: " + tracker.getName());
> 506:         var error = TRACKER.check(tracker, 10000);

Hello Darragh, 10 seconds looks a bit too large, even if it's the max time to wait. Before this change the graceDelayms was 500 milli seconds. So this appears to be a big jump. Perhaps we should start with something like 2 or 5 seconds (an arbitrary number, I admit)?

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

PR Review Comment: https://git.openjdk.org/jdk/pull/15307#discussion_r1302575954
PR Review Comment: https://git.openjdk.org/jdk/pull/15307#discussion_r1302573457