RFR: 8301243: java/net/httpclient/http2/IdleConnectionTimeoutTest.java intermittent failure

[Daniel Fuchs]

On Tue, 7 Feb 2023 16:52:43 GMT, Conor Cleary <ccleary at openjdk.org> wrote:

>> test/jdk/java/net/httpclient/http2/IdleConnectionTimeoutTest.java line 179:
>> 
>>> 177:                     int cmp = Integer.compare(firstConnectionHash, secondConnectionHash);
>>> 178: 
>>> 179:                     if (cmp < 0 | cmp > 0) {
>> 
>> In other words, `if (cmp != 0)` ? ;-)
>> 
>> It may be simpler and less error prone to save the connection (as an Object) in the handler.
>> 
>>         volatile Object previousConnection = null;
>> 
>> 
>> Then the condition become:
>> 
>>                 if (previousConnection == null) {
>>                     previousConnection = exch.getTestConnection();
>>                     exch.sendResponseHeaders(200, 0);
>>                 } else {
>>                     var secondConnection = exch.getTestConnection();
>> 
>>                     if (previousConnection != secondConnection) {
>>                         ....
>
> I could be wrong but I think I might have tried that before. What you're calling `previousConnection` in the sample code you've given gets set to `null` in the time between the first and second requests during the tests where a timeout fires. Which does serve to test that the connections are different to be fair but maybe for some reason `previousConnection` could be null that is unrelated to it being closed by the test server. For example `createConnection()` might close early due to a Reset Stream or Protocol Error. 
> 
> I know that's very edge case but its the reason I compared using the hash code. I'm open to changing it though of course! Might be less error prone as you say.
> 
> Also, very fair on the `if (cmp != 0)` suggestion 😅 I'll definitely shorten it to that if I stick with the comparison as it is right now.

I don't see how using the hash wouldn't cause the exact same issue. Also you really need volatile here, since two different requests are going to be handled by two different threads.

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

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