mirror of
https://github.com/hanzoai/datastore.git
synced 2026-09-26 13:55:07 +00:00
Alexey MilovidovandClaude Opus 4.8
6c72e9a563
`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]>