Repository navigation
Discard pooled connections the server closed while idle - #968
Merged
Conversation
Asks the kernel what it already knows about the socket, so a connection the server closed while idle can be spotted without any I/O. Poll(SelectError) catches failed connects and out-of-band state; readable-with-nothing-to-read is EOF, which is what a FIN or RST leaves behind. Both arms independently detect FIN and RST on macOS, so the tests cannot discriminate between them there. Winsock sets exceptfds only for out-of-band data and failed connects, never for a peer's graceful close, so the EOF arm is what carries FIN detection on Windows.
Plumbs SystemReportsDead() from TcpSocketClient up to IConnection and consults it in ConnectionValidator, so a connection the server closed while idle is destroyed at checkout instead of being handed out. It goes last in the conjunction: every other check is in-memory arithmetic, so the syscall only runs on a connection that has passed all of them, and IsOpen short-circuiting ahead of it keeps a disposed socket away from Poll. ConnectionPool is unchanged - returning Unhealthy already means destroy and retry, which is the agreed failure handling: drop the connection, leave the routing table alone.
ADR 021: "New connections are never checked against the timeout. The successful handshake and version negotiation is considered enough." The check was running on freshly created connections too, which broke test_disconnect_after_hello - the driver noticed the death at acquire time and surfaced the error from session.run() instead of from the first next(). Provenance has to be threaded out of the pool because both paths can return an idle connection: CreateNewConnectionOrGetIdleAsync falls back to the idle set when the pool is full, so it cannot be inferred from which method ran. Checking a connection created microseconds ago could never find anything, and against a server that closes every connection immediately the acquire loop would have spun until the acquisition timeout instead of surfacing the error on first use.
RichardIrons-neo4j
force-pushed
the
drivers-576-dotnet-check-connection-state-when-taken-from-connection
branch
from
September 25, 2026 14:59
478eda9 to
76a26dc
Compare
RichardIrons-neo4j
requested review from
AndyHeap-NeoTech
and removed request for
AndyHeap-NeoTech
September 30, 2026 09:05
AndyHeap-NeoTech
requested changes
Oct 2, 2026
|
|
||
| public bool SystemReportsDead() | ||
| { | ||
| if (_client.Poll(0, SelectMode.SelectError)) |
Contributor
There was a problem hiding this comment.
What happens if _client.Poll or _client.Available throw? My hunch is that the connection is leaked because it has already been taken from the idle pool but never destroyed. Potentially the pool slot is lost as well depending on when the size counter is decremented.
I suggest any failure should be treated as 'dead'.
Contributor
Author
There was a problem hiding this comment.
Good catch, wrapped in a try block now (with a regression test).
A peer that leaves bytes in the buffer before closing is still dead, but the EOF check can't see it: Available is non-zero. On Unix it passes only because Poll(SelectError) reports the close; Winsock sets exceptfds for out-of-band data and failed connects, never a graceful close, so this is the case that would fail on Windows. A TLS close_notify sent before the FIN is the realistic way to get here.
Readable-and-empty was the EOF test, but anything left unread in the buffer masks it: a TLS close_notify sent before the FIN makes Available non-zero and the connection looked alive. A connection only reaches the pool after a clean RESET exchange, so it has nothing pending and the server has nothing to say to it; if it becomes readable, the peer has gone or something unexpected arrived, and either way it should not be handed out. That also makes the SelectError arm redundant. A closed, reset or errored socket reports readable on both POSIX and Winsock, and SelectError was the arm whose behaviour differs between them - Winsock never reports a graceful close there. The risk is a healthy server talking to an idle connection, which would now get it discarded on every checkout. Per the Bolt spec NOOPs are only sent while the server is processing a request, but that has not been measured.
…k-connection-state-when-taken-from-connection
If SystemReportsDead() threw, the exception escaped AcquireAsync after the connection had been taken from the idle set, so the destroy path never ran and the pool slot was lost for good.
The validator only checks connections that came from the idle set. Cover the three ways AcquireAsync can obtain one: idle, newly created, and an idle connection taken while waiting on a full pool.
Enables the stub test covering connections the server closes while idle.
Covers a healthy encrypted connection staying silent after a round trip and a close_notify with the socket left open, for TLS 1.2 and 1.3. TLS 1.3 is skipped where the platform doesn't support it.
RichardIrons-neo4j
added a commit
that referenced
this pull request
Oct 22, 2026
* Backport SEC-01 and SEC-04 from the 6.x security report fixes (#958) CustomAuthToken.ToString() masks credentials and parameters. Every trust manager refuses a peer that presented no certificate. * Backport discarding pooled connections the server closed while idle (#968) The validator asks the OS whether a connection taken from the idle pool is dead and evicts it instead of handing it out.
This was referenced Oct 24, 2026
This was referenced Oct 31, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes DRIVERS-576
Before handing out a pooled connection the driver now asks the OS whether it is already dead. If it is, the driver drops the connection and leaves the routing table alone.