Skip to content

Fail closed after an interrupted async exchange - #37

Open
chenty2333 wants to merge 1 commit into
skytable:nextfrom
chenty2333:codex/cancelled-connection-poison
Open

chenty2333 wants to merge 1 commit into
skytable:nextfrom
chenty2333:codex/cancelled-connection-poison

Conversation

@chenty2333

Copy link
Copy Markdown

A cancelled query or pipeline can leave its reply unread. Reusing the connection then returns that reply to the next request. Pool checkout validation can consume a stale reply too, so validation alone does not prevent the mismatch.

Track incomplete exchanges with a drop guard. Later operations return ConnectionAborted without writing, and both TCP and TLS pool managers discard the poisoned connection. Direct users must reconnect. Complete replies, including server errors, remain reusable; dropping an unpolled future does not poison the connection. No public API is added.

The patch does not undo or replay the cancelled request: it may already have executed on the server.

Validation:

  • 17 library tests pass, including cancellation, partial write/read, EOF, protocol errors, completed replies and unpolled futures.
  • cargo check --all-targets, formatting and diff checks pass.
  • A separate live test against Skytable 0.8.4 reproduces stale replies with the unmodified 0.8.12 client. With this patch, direct TCP/TLS connections reject reuse and their pools return fresh connections. The TCP pool test also covers size 2, a concurrent borrower, and checkout validation disabled.

A cancelled query or pipeline can leave an unread reply on the connection. Reusing it associates that reply with the next request, including when checkout validation consumes the stale reply first.

Poison incomplete exchanges and reject later operations without writing. Have both async pool managers discard poisoned connections, and document that direct users must reconnect. Complete responses, including server errors, remain reusable.

Add regression coverage for cancellation, partial reads/writes, EOF, protocol errors, unpolled futures and completed replies.

Assisted-by: LLM
Signed-off-by: Tianyi Chen <hi@tychen.cc>
@chenty2333

Copy link
Copy Markdown
Author

I'm happy to sign the CLA, but the signing page reports 'Bad credentials' and says there is no CLA to sign. Could you please check the CLA service configuration? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant