RFR: 8300268 : ServerImpl allows too many idle connections when using sun.net.httpserver.maxIdleConnections [v3]

[Jaikiran Pai]

On Tue, 7 Feb 2023 15:40:48 GMT, Darragh Clarke <duke at openjdk.org> wrote:

>> Currently there is a race condition that can allow for too many 'idleConnections' in `ServerImpl`
>> 
>> This PR adds a lock to make sure only one connection can be marked Idle at a time as well as a test that consistently failed before the change but which now passes.
>
> Darragh Clarke has updated the pull request incrementally with one additional commit since the last revision:
> 
>   addressed comments

src/jdk.httpserver/share/classes/sun/net/httpserver/ServerImpl.java line 413:

> 411:                     }
> 412:                     responseCompleted (c);
> 413:                     if (t.close || idleConnections.size() >= MAX_IDLE_CONNECTIONS) {

The root cause of the issue appears to be that, before this change, the code execution, at time T1, would notice that the `idleConnections` set hasn't yet reached the configured limit and would thus skip the closing of the connection. However, it wouldn't immediately add this unclosed connection to the `idleConnections` Set and instead would add it to a temporary collection (the  `connsToRegister`) and at a later time T2 would then iterate over the temporary `connsToRegister` collection and mark the connections in that collection as idle. That would thus leave a (considerable) period of time between T1 and T2 where some other threads could reach this same code and notice that the idle connection set hasn't yet reached the limit, but they wouldn't consider the connections that have been temporarily added to the `connsToRegister` set, in this limit computation. Thus it would lead to a situation where, when the connections are ultimately added to the `idleConnections` set, in `markIdle(
 ) method`, they would potentially exceed the limit.

The change in this PR proposes that `markIdle()` method take the responsibility of checking the idle connection limit and deciding whether or not to accomodate the connection in that Set. I think, that's a good thing.

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

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