On Thu, 25 Apr 2024 08:04:16 GMT, Christoph Langer <clan...@openjdk.org> wrote:

>> 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.
>
> I've thought about this, too. However, I see the only critical point why it 
> could merit a `/othervm` test is this thing with querying the thread CPU time 
> of the Keep-Alive-SocketCleaner thread. But I think the likelihood of this 
> failing is the same within standard test and `/othervm`. So I'd prefer to 
> change to standard test, since this is one of the improvements of this change.

The cache is global. Who knows what it might already contain when the test 
starts and what may be left behind when the test ends. So I agree that it may 
be prudent to keep `/othervm`

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

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

Reply via email to