`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]>
* Replicate poco into base/poco/
* De-register poco submodule
* Build poco from ClickHouse
* Exclude poco from stylecheck
* Exclude poco from whitespace check
* Exclude poco from typo check
* Remove x bit from sources/headers (the style check complained)
* Exclude poco from duplicate include check
* Fix fasttest
* Remove contrib/poco-cmake/*
* Simplify poco build descriptions
* Remove poco stuff not used by ClickHouse
* Glob poco sources
* Exclude poco from clang-tidy