RFR: 8293562: blocked threads with KeepAliveCache.get [v5]
Michael McMahon
michaelm at openjdk.org
Fri Sep 30 10:42:32 UTC 2022
On Wed, 28 Sep 2022 17:31:46 GMT, Daniel Jeliński <djelinski at openjdk.org> wrote:
>> Please review this patch that makes sure KeepAliveCache does not block all threads while closing sockets.
>>
>> Changes:
>> - get operation no longer closes sockets; if there's no socket that is recent enough, get returns null and lets the cleaner thread close the sockets
>> - put operation closes sockets without holding the cache lock. Additionally, if the cache is full, it places the new connection in the cache and removes the oldest connection.
>> - the cleaner thread creates a list of connections to close, and then closes them after releasing the cache lock
>> - additionally, we set the socket timeout to 1 millisecond before calling socket.close
>>
>> The new test fails with `Wait for second request timed out` without this patch, passes after the changes. Tiers 1-3 clean.
>
> Daniel Jeliński has updated the pull request incrementally with one additional commit since the last revision:
>
> Describe oldClient
src/java.base/share/classes/sun/net/www/http/KeepAliveCache.java line 330:
> 328: long currentTime = System.currentTimeMillis();
> 329: KeepAliveEntry e = peekFirst();
> 330: if ((currentTime - e.idleStartTime) > nap) {
Since `peekFirst` can return null in theory, should we check for it? Currently `pop` is used which throws an exception instead, and probably means it doesn't happen in practice, but maybe it could be flagged in static source code analysis?
-------------
PR: https://git.openjdk.org/jdk/pull/10401
More information about the net-dev
mailing list