Repository navigation
pg: a timed-out connection is shut down, and freed only once its armed recv has returned - #292
Merged
Merged
Conversation
…ts armed recv has returned A command timeout disposed the connection while its reader was parked on a ring recv: close(fd), then the buffers freed. close() does not end a socket an io_uring op still holds, so the socket stayed ESTABLISHED and the recv stayed armed until the server finally answered, and the kernel then wrote that reply into the freed receive buffer - which the allocator had already handed to the replacement connection (same address, same fd number). Pick's dispose of a connection broken by one loop could hit the other loop's op the same way. Dispose now shuts the socket down while a loop is parked on an op, which completes it, and the loop that exits last closes the fd and frees the buffers. Dispose marks the connection broken, so it takes no new commands in between. "pg:" and "redis: a timed-out connection's socket ends at the timeout, not when the late reply comes" time a command out on the sidecar (pg_sleep(30), BLPOP 30) and require the pool connection that timed out to leave ESTABLISHED within 10 s, while its sibling stays ESTABLISHED and the pool still answers. Confirmed both ways: both fail on the unmodified source (still ESTABLISHED after 10 s) and pass with the fix.
Owner
Author
|
Bench, before/after:
Within the noise floor: Pg's median is 2% higher, but the first round measured the same (5.46 vs 5.47 us), and Redis, which has the identical change, is flat. Method: |
Merged
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.
Now pg-only. The Redis half went with
ioxide.redis, which #337 drops; main is merged in, and the Pg suite passes (7).A pg command that timed out left its connection's socket open, with a recv still armed against a receive buffer that had already been freed.
On a command timeout the pool disposed the connection while its reader was parked on a ring recv:
close(fd), thenNativeMemory.Freeof the send and receive buffers.close()does not end a socket that an io_uring operation still holds. So the socket stayed ESTABLISHED and the recv stayed armed until the server finally answered, and the kernel then wrote the late reply into the freed receive buffer. In the reproduction, the allocator had already handed that buffer (same address) and the same fd number to the replacement connection, so the reply landed in a live connection's memory.Pickdisposing a connection that one loop had marked broken could hit the other loop's operation the same way.Disposenow marks the connection broken and, while a loop is parked on an operation, callsshutdown(fd, SHUT_RDWR)and returns. That completes the operation, and the loop that exits last runsDisposeagain, which closes the fd and frees the buffers. Both entry points andPickalready refuse a broken connection, so nothing touches the buffers in between. It reuses the existing_reading/_sending/IsBrokenstate. Two comments that said closing the fd cancels the stuck recv are corrected.Tests
New:
pg: a timed-out connection's socket ends at the timeout, not when the late reply comes(pg_sleep(30))It uses a pool of 2 and a 1000 ms command timeout. The test requires the timed-out connection's socket to leave ESTABLISHED within 10 s. The sibling connection must stay ESTABLISHED and the pool must still answer afterwards. The socket states come from a new harness helper,
TcpSockets, which reads/proc/net/tcpand counts only this process's sockets._received65 at ~5.2 s in apg_sleep(5)probe)Suites: Pg 7 with main merged in (2026-10-12); Unit 63 when first run.
Two behaviour notes:
Disposeis then a no-op.Disposecalled after the reactor has stopped, while a loop was still parked, now leaves that connection's fd and buffers to process exit rather than freeing them under the parked operation.Bench: still to do. This is a pg module change, so it gets a wrk before/after, which I'll post here once the box is quiet; other suites are running on it now. The only hot-path cost is one bool check when a loop exits.