Repository navigation
Close connections after incomplete meta operations - #91
Merged
Merged
Conversation
sam-obeid
approved these changes
Sep 24, 2026
mrattle
force-pushed
the
c1-cancellation-safety
branch
from
September 24, 2026 15:37
7b4e044 to
2a3c114
Compare
mrattle
force-pushed
the
c1-cancellation-safety
branch
2 times, most recently
from
September 24, 2026 16:17
d4aa8b1 to
17e1420
Compare
mrattle
pushed a commit
that referenced
this pull request
Oct 16, 2026
PR #91 introduced OperationGuard for the meta commands only. The ASCII commands and the client-level version, stats, flush_all and dump_keys still wrote a request and read the reply with nothing marking the connection unusable if the future was dropped in between, so the stale reply became the answer to the next request on that connection. Wrap all 18 remaining entry points in the same guard, following the rule already used by the meta commands: validate key length before starting the operation so a rejected key sends nothing and keeps the connection; complete the guard after the last read and before matching the response so fully read refusals and error lines never close it; for no-reply commands, complete right after flush. MetadumpIter now owns the guard as an Option. END, BADCLASS and BUSY complete it and end the iterator, so a refusal leaves the connection usable; a network or protocol error, or dropping the iterator before END, drops the guard uncompleted and closes the connection. BUSY was previously non-terminal and re-read the socket on every next() call. The ASCII module reuses the crate-level MAX_KEY_LENGTH instead of a duplicate constant. Doc comments and the changelog no longer say the close applies to meta operations only. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
mrattle
force-pushed
the
c1-cancellation-safety
branch
from
October 16, 2026 17:20
17e1420 to
c527407
Compare
mrattle
pushed a commit
that referenced
this pull request
Oct 17, 2026
PR #91 introduced OperationGuard for the meta commands only. The ASCII commands and the client-level version, stats, flush_all and dump_keys still wrote a request and read the reply with nothing marking the connection unusable if the future was dropped in between, so the stale reply became the answer to the next request on that connection. Wrap all 18 remaining entry points in the same guard, following the rule already used by the meta commands: validate key length before starting the operation so a rejected key sends nothing and keeps the connection; complete the guard after the last read and before matching the response so fully read refusals and error lines never close it; for no-reply commands, complete right after flush. MetadumpIter now owns the guard as an Option. END, BADCLASS and BUSY complete it and end the iterator, so a refusal leaves the connection usable; a network or protocol error, or dropping the iterator before END, drops the guard uncompleted and closes the connection. BUSY was previously non-terminal and re-read the socket on every next() call. The ASCII module reuses the crate-level MAX_KEY_LENGTH instead of a duplicate constant. Doc comments and the changelog no longer say the close applies to meta operations only. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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
2 participants
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.
TL;DR - What are you trying to accomplish?
Prevent incomplete meta operations from leaving reusable connections with unread responses or partial commands. This covers all existing meta protocol APIs (including the newly added multi-ops).
Details - How are you making this change? What are the effects of this change?
Each of the seven meta operations holds a guard from its first write through response completion, including quiet-mode and batch
MNhandling. Cancellation or an early error drops the socket and its buffers without flushing pending writes. A batch that returns an error line beforeMNalso closes the connection. The operation is not retried.Parsed response bytes are consumed immediately, removing the stale
last_read_nstate. Preflight validation failures and fully consumed refusals keep the connection available for reuse.Client::is_closed()exposes the local closed state so callers can replace the client. Later I/O on a closed client, from meta or ASCII methods, returns the newError::ConnectionClosed. Other I/O failures remainError::Io. The ASCII implementation is unchanged.Breaking:
Error::ConnectionClosedis a new variant. Exhaustive matches onErrormust handle it.