On Fri, 5 Aug 2022 06:29:42 GMT, Jaikiran Pai <[email protected]> wrote:

>> Hi,
>> 
>> Some new keep alive tests are exposing some old bugs. In this case if the 
>> server sends an invalid timeout (say -20 seconds) we accept it creating a 
>> timeout in the past. So, the first time the keep alive thread wakes up it 
>> will close the connection.
>> The correct behavior is to ignore the invalid parameter and fallback to the 
>> default timeout or the timeout set by the relevant system property.
>> 
>> Thanks,
>> Michael
>
> test/jdk/sun/net/www/http/KeepAliveCache/B8291637.java line 27:
> 
>> 25:  * @test
>> 26:  * @bug 8291637
>> 27:  * @run main/othervm -Dhttp.keepAlive.time.server=20 -esa -ea B8291637
> 
> Is it intentional that we are setting the -esa and -ea options here?

If we enable assertions without the bug fix, then an assertion is triggered in 
KeepAliveCache, which is useful.

> test/jdk/sun/net/www/http/KeepAliveCache/B8291637.java line 103:
> 
>> 101:                 }
>> 102:             } catch (IOException e) {
>> 103:                 System.err.println("Server exception terminating");
> 
> It looks to me that if an exception occurs (not just `IOException`) then this 
> test might hang (and timeout with the jtreg timeout) at the `passed.get()` 
> call in the `main()` method. Should we perhaps catch `Throwable` here and do 
> `passed.completeExceptionally`?

Seems reasonable.

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

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

Reply via email to