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

[Michael McMahon]

On Tue, 22 Nov 2022 12:22:57 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 two additional commits since the last revision:
> 
>  - Update test/jdk/sun/net/www/http/HttpClient/MultiThreadTest.java
>    
>    Co-authored-by: Andrey Turbanov <turbanoff at gmail.com>
>  - Update test/jdk/sun/net/www/http/HttpClient/MultiThreadTest.java
>    
>    Co-authored-by: Andrey Turbanov <turbanoff at gmail.com>

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

> 81:     // The number of time an idle timeout was detected by any worker
> 82:     // on the server side.
> 83:     static final AtomicInteger IDLE_TIME_EXCEEDED = new AtomicInteger();

IDLE_TIME_EXCEEDED is allocated and incremented, but not referenced anywhere else. Did you mean to include it in a debug log or something?

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

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