On Tue, 31 Oct 2023 09:28:23 GMT, Michael McMahon <[email protected]> wrote:
>> This patch fixes a race between the selector thread and the IdleTimeoutTask
>> which closes connections that were idle for longer than IDLE_INTERVAL. With
>> this patch, only the thread that successfully removes the connection from
>> idleConnections is permitted to change it. If any data arrives after the
>> IdleTimeoutTask removes the connection from idleConnections, the data is
>> ignored.
>>
>> Additionally, this patch reduces the time interval between when the
>> IdleTimeoutTask removes the connection from idleConnections and when the
>> connection is closed, back to pre-JDK-8286918 level.
>>
>> No new test; with platform sockets it's next to impossible to reliably get
>> the timing right. Existing tier1-3 tests pass.
>
> src/jdk.httpserver/share/classes/sun/net/httpserver/ServerImpl.java line 1032:
>
>> 1030: ArrayList<HttpConnection> toClose = new ArrayList<>();
>> 1031:
>> 1032: connections.forEach(c -> {
>
> Shouldn't we still be synchronizing on `connections` as per the advice in
> `Collections::synchronizedSet` ?
It's only necessary for iterator and stream; forEach (the non-stream version)
is internally synchronized.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/16366#discussion_r1377324101
