On Tue, 23 Apr 2024 08:39:56 GMT, Christoph Langer <clan...@openjdk.org> wrote:

>> While working in that area I found some potential for cleanup of a few tests.
>> 
>> Most notably:
>> 
>> B5045306.java:
>> - does not need to run in othervm
>> - the executor service that it uses should be shut down eventually to free 
>> resources
>> 
>> B8291637.java:
>> - use just one instead of two test VM invocations
>> KeepAliveTimerThread.java:
>> call to grp.destroy() at the end is pointless (API is void & deprecated for 
>> removal)
>> 
>> Generally:
>> The deprecated URL constructor is used. It can be switched to the handy 
>> URIBuilder
>> Some more try with resources here and there
>
> Christoph Langer has updated the pull request incrementally with one 
> additional commit since the last revision:
> 
>   Update test/jdk/sun/net/www/http/KeepAliveCache/B5045306.java
>   
>   Co-authored-by: Andrey Turbanov <turban...@gmail.com>

test/jdk/sun/net/www/http/KeepAliveCache/B5045306.java line 154:

> 152: 
> 153:                     // if Keep-Alive-SocketCleaner consumes more than 
> 50% of cpu then we
> 154:                     // can assume a recursive loop.

Interesting test case. I'm a bit surprised we haven't seen this intermittently 
fail. Speaking of which, I see that the change to this test proposes to remove 
it from `/othervm` and potentially run it in agent vm mode. Given the kind of 
checks this test is doing, I think it's better to let it run as a `othervm` 
test to keep the test as isolated as possible.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/18884#discussion_r1577380954

Reply via email to