RFR: 8223783: sun/net/www/http/HttpClient/MultiThreadTest.java sometimes detect threads+1 connections [v6]

[Michael McMahon]

On Wed, 23 Nov 2022 14:25:37 GMT, Daniel Fuchs <dfuchs at openjdk.org> wrote:

>> Please find here a test fix for the sun/net/www/http/HttpClient/MultiThreadTest.
>> 
>> This test makes concurrent connections to a server using multiple threads, and due to keep-alive, expects that there should not be more connections than concurrent threads. However, the test has been seen occasionally and intermittently failing because it observed more connections than threads. The test has some built-in logic to exclude connections that do not come from the test itself, so external interference should not be the cause.
>> 
>> The suspicion is that occasionally some connection might remain idle for more than the keep-alive timeout, which then causes it to be closed, and allows the client to open a new connection.
>> 
>> This fix works this way: 
>> 
>> - first it adds some logic to figure out whether a Worker thread might have remained idle for more than the keep-alive timeout
>> - then it takes this information into account when matching the number of actual connections with the number of expected connection,
>> - additionally it reduces the keep-alive timeout to one second, and forces some sleep on the client side after half the requests have been sent to increase the probability that some connections will be idle long enough to trigger the idle-time detection logic (as a means to test the fix itself). 
>> 
>> Finally it improves logging by adding more diagnosis (including human readable time stamps), so that future failures, if any, will be easier to diagnose. Note that reducing the keep-alive time to 1 second makes the test run much faster even with the 1500ms sleep on the client side.
>> 
>> With this fix I no longer observe the test failing.
>
> Daniel Fuchs has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Use the tests list to simplify the logic of waiting for clients to finish

test/jdk/sun/net/www/http/HttpClient/MultiThreadTest.java line 193:

> 191:             // our client. We should only count those workers for which
> 192:             // the expected header has been found.
> 193:             int validConnections = 0;

Could you also add a comment like:

// We detect worker threads that may have timed out, so we don't include them in
// the count to compare with the number of connections.

Otherwise, LGTM

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

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