On Mon, 30 Oct 2023 06:17:04 GMT, Jaikiran Pai <[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 531: > >> 529: // has now started on this >> connection. >> 530: requestStarted(conn); >> 531: handle (chan, conn); > > In its current proposed form, if the connection wasn't in the > newlyAcceptedConnections or idleConnections set, we don't seem to do anything > (and expect the connection to be closed in a concurrent thread). Perhaps we > should move this if block a few lines above and only cancel the key and put > the channel in blocking mode, if this `if` condition satisfies? It's complicated, and perhaps deserves a separate fix. If the connection is plain HTTP, it doesn't matter; we close the connection, and this succeeds regardless if the connection is blocking or not. If the connection is HTTPS, we send an alert to the peer using this code: https://github.com/openjdk/jdk/blob/3934127b087ade1c1286008df3497ca6d84778a5/src/jdk.httpserver/share/classes/sun/net/httpserver/SSLStreams.java#L294-L296 And here again the blocking vs. non-blocking mode doesn't matter: if there's room in the send buffer, the send will succeed immediately, and if there's no room, it will block. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/16366#discussion_r1375881139
