Repository navigation
Close connections after incomplete ASCII and admin operations - #93
Merged
Merged
Conversation
mrattle
force-pushed
the
c1-cancellation-safety
branch
3 times, most recently
from
September 24, 2026 16:17
d4aa8b1 to
17e1420
Compare
mrattle
force-pushed
the
cancellation-safety-all-entry-points
branch
from
October 16, 2026 17:20
feba5b0 to
e4ea568
Compare
mrattle
force-pushed
the
c1-cancellation-safety
branch
from
October 16, 2026 17:20
17e1420 to
c527407
Compare
Extend the fake-server suite from the seven meta operations to all 25 public entry points. Each replying ASCII command, plus version, stats, flush_all and dump_keys, is cancelled between request and response and must leave the client closed; a closed client must refuse every entry point without touching the socket; completed operations, fully read refusals, no-reply commands, key-length rejections and unpolled futures must all leave the connection open and in sync. The metadump iterator gets its own cases: dropping it mid-dump closes the connection, END/BADCLASS/BUSY end the iterator and keep the connection, and cancelling a single next() call leaves the partially read entry buffered for the following call. Seven of these tests fail on purpose until the guard is extended; the next commit turns them green. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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
cancellation-safety-all-entry-points
branch
from
October 17, 2026 18:41
e4ea568 to
58e390b
Compare
nherson
approved these changes
Oct 19, 2026
mrattle
approved these changes
Oct 19, 2026
mrattle
left a comment
mrattle
left a comment
Contributor
There was a problem hiding this comment.
Original changes looked good, patches were tested through staging and behave as expected.
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
3 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.
What
Extends #91's
OperationGuardfrom the meta commands to every entry point, so cancelling or failing any operation mid-response closes the connection instead of leaving a stale reply for the next request.Changes
AsciiProtocolmethod (and_no_replyvariants), plusversion,stats,flush_allanddump_keys. Key validation stays before the guard, soKeyTooLongleaves the connection usable.version/flush_all: a server closing mid-reply now returnsIo(UnexpectedEof)and closes the client, like every other entry point (previously a protocol error withis_closed() == false).MetadumpIterskipped withmem::forget/Box::leakno longer lets its unread reply answer the next request.is_closed()reports it, and the next call closes the connection and returnsConnectionClosed.MetadumpIter:ENDcloses the connection immediately.Some(Err(_))is terminal (includingBUSY); newis_complete()tells a finished dump from one cut short.flags=and a trailing space aftersize); a non-empty cache previously failed to dump.Error::ConnectionClosed(from Close connections after incomplete meta operations #91).Breaking changes
MetadumpIterkeeps theClientmutably borrowed until it is dropped, even afternext()returnsNone. Drain-then-reuse in the same scope now fails with E0499; drop the iterator or end its scope first (astests/client_integration_tests.rsnow does withdrop(dump)).Error::ConnectionClosed(from Close connections after incomplete meta operations #91): treat it likeIoand discard the client; checkis_closed()in pool recycle /has_broken.Cut the next release as 0.9.0 after this lands.
Testing
New tests in
tests/cancellation_safety.rscover cancellation, EOF, leaks and metadump termination for each entry point; a new real-server test dumps a populated cache.scripts/test-integration.sh,cargo fmt --checkandcargo clippy --all-targets --all-features -- -D warningspass.