Context
The driver's internal topology.connection logic should be refactored to include a persistant bufio.Reader. There motivation for this is to safely use peek. More specifically, GODRIVER-3173 will, under certain conditions, require performing an aliveness check before attempting to drain the buffer on a TCP connection that previously failed due to a socket timeout. Currently there is no way to do that without discarding bytes from the TCP buffer, which can cause problems with subsequently reading the size.
Definition of done
- Add a single br *bufio.Reader field to the topology.connection struct
- Initialize this reader exactly once within the connect() method, immediately after the underlying net.Conn (nc) is fully established (post-TLS handshake)
- Refactor all methods that read from the connection, specificially read() and awaitPendingResponse(), to use the shared conn.br
- Add a test for the custom dialer case--for example users will expect a 2 part read: one for the size and one for the rest of the message. Likely the size check will still need to use c.nc since c.br will by default try to do a large internal fill (e.g. 4096).
This change centralizes all read operations through a single, long-lived buffered reader, ensuring data integrity and optimizing network performance.
Here are the error cases for using bufio#Reader.Peek:
| Scenario | Error from Peek | Reusable? | IsAlive returns | Notes |
|---|---|---|---|---|
| Peer sent FIN (clean close, peer process crash) | io.EOF | No | false | Standard "peer hung up" signal |
| Peer sent RST (process killed, port closed forcibly) | *net.OpError wrapping syscall.ECONNRESET | No | false | Timeout() == false |
| Network drop, no FIN/RST received yet | os.ErrDeadlineExceeded | Yes (false negative) | true | Local stack hasn't noticed; indistinguishable from alive-but-idle until keepalive fires |
| Network drop after kernel TCP keepalive failure | *net.OpError with non-timeout cause | No | false | Default keepalive interval is multi-minute |
| TLS-layer abort (fatal alert, record header error) | tls.RecordHeaderError / tls.AlertError / similar | No | false | Not wrapped in *net.OpError; check explicitly |
| Local side called Close() | net.ErrClosed | No | false | "use of closed network connection" |
| Alive but idle (no data within probe deadline) | os.ErrDeadlineExceeded | Yes | true | The expected non-error during a liveness probe |
| MongoDB closed a cursor; TCP is still healthy | nil (returns peeked bytes) or unexpected response | Maybe | true | TCP-layer is alive; protocol-layer state is the cursor owner's job |
See here for playground: https://github.com/prestonvasquez/go-playground/blob/main/net_peek_test.go
Pitfalls
bufio.Reader is not safe for concurrent use, this design relies on the assumption that the connection pol realizes access given to a connection object. While this is a spec-requirement, if there is any flaw in the pool's locking or checkout logic that allows two goroutines to read from the same connection simultaneously, it will lead to mangled data and likely difficult-to-diagnose race conditions.
IsAlive cannot detect a silent network drop the kernel hasn't noticed yet. Such connections are reported as alive until keepalive fires (default multi-minute). Inherent limit of any non-keepalive probe at the application layer.
| Failure mode | Closed() catches | IsAlive() catches |
|---|---|---|
| Peer sent FIN (clean close) |  |
 |
| Peer sent RST | |
|
| Server-side close | |
|
| TLS abort | |
|
| Local code called Close() | |
|
| Network drop, kernel notified (keepalive fired) | |
|
| Network drop, kernel hasn't noticed | |
|
- has to be done before
-
GODRIVER-3173 CSOT avoid connection churn when operations timeout
-
- Backlog
-
- is depended on by
-
GODRIVER-3905 Replace mnet Close/Closed with IsAlive
-
- Blocked
-
- is duplicated by
-
-
- Closed
-
- is related to
-
-
- Blocked
-