Alexey MilovidovandClaude Opus 4.8 6c72e9a563 Detect HTTP keep-alive peer disconnect with MSG_PEEK/SSL_peek, not poll ...

`HTTPConnectionPool::isStale` decided whether a pooled keep-alive
connection was still usable with `poll(SELECT_READ, 0)`: readable meant
stale, on the assumption that an idle keep-alive connection has no data
pending. On a plain socket that holds, but on an HTTPS connection it
produces a false positive. `poll` reflects the raw TCP socket, so an
unread TLS post-handshake record (a session ticket or `KeyUpdate`) makes
an entirely live secure connection look readable, i.e. stale, and the
pool discards it and reconnects - at the cost of a fresh TLS handshake -
on nearly every borrow.

This ports the shared, TLS-aware helper `isSocketPeerClosed` (added in
`src/IO`) from the distributed cache fix and switches
`HTTPConnectionPool::isStale` to it:

- for a plain socket, a single non-blocking, non-destructive
  `recv(..., MSG_PEEK | MSG_DONTWAIT)`: `0` means the peer sent a FIN
  (closed); `> 0` means bytes are on the wire (alive); `EAGAIN` /
  `EWOULDBLOCK` means a healthy idle connection; any other error (e.g.
  `ECONNRESET`) is treated as closed/broken;
- for a TLS (`SecureStreamSocket`) socket, an `SSL_peek` on a temporarily
  non-blocking socket. A raw `MSG_PEEK` cannot tell TLS records apart -
  an unread `close_notify` looks the same as unread application data or a
  session ticket, and in TLS 1.3 the record content type is encrypted.
  `SSL_peek` decrypts just enough to distinguish real application data and
  harmless post-handshake messages (a session ticket or `KeyUpdate`),
  treated as alive, from a `close_notify` (`SSL_ERROR_ZERO_RETURN`) or a
  broken session, treated as closed, without consuming application data.

A small `ssl` accessor is added to Poco's
`SecureSocketImpl` / `SecureStreamSocketImpl` to reach the underlying
OpenSSL `SSL` object.

`gtest_socket_peer_closed` covers both paths, including a
`socketpair`-backed TLS connection: an unread session ticket and pending
application data read as alive; a `close_notify` and an abrupt close read
as closed. The `close_notify` case also asserts that a raw `MSG_PEEK`
misfires there, which is exactly why the TLS-aware path is needed.

This is the public-repo counterpart, and the follow-up promised in, the
distributed cache fix https://github.com/ClickHouse/clickhouse-private/pull/64217

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

2026-07-14 14:26:39 +00:00

..

Net

Detect HTTP keep-alive peer disconnect with MSG_PEEK/SSL_peek, not poll

2026-07-14 14:26:39 +00:00