Skip to content

Add Connection::stream_send_complete() - #2571

Open
kixelated wants to merge 1 commit into
cloudflare:masterfrom
kixelated:feat/stream-send-complete
Open

Add Connection::stream_send_complete()#2571
kixelated wants to merge 1 commit into
cloudflare:masterfrom
kixelated:feat/stream-send-complete

Conversation

@kixelated

Copy link
Copy Markdown

(AI generated but reviewed)

quiche can report that the peer finished sending on a stream (stream_finished), but not that the peer acknowledged what we sent. The send-side state exists internally as SendBuf::is_complete(), where it gates stream collection, but nothing exposes it.

Without it an application cannot tell a stream whose FIN was merely queued from one the peer actually received. That matters before a reset: RESET_STREAM on a stream still in DataSent discards the unacknowledged data, so a final message can be dropped even though stream_send() reported it written. Callers that want a delivery guarantee currently have to avoid resetting streams entirely, or invent an application-level acknowledgement.

Report is_shutdown()/is_stopped() streams as incomplete rather than deferring to is_complete(). A reset counts as complete for collection purposes, since there is nothing left to send either way, but as an answer to "did the peer get my FIN" it would be wrong.

Fixes #1722

quiche can report that the peer finished sending on a stream
(stream_finished), but not that the peer acknowledged what we sent. The
send-side state exists internally as SendBuf::is_complete(), where it
gates stream collection, but nothing exposes it.

Without it an application cannot tell a stream whose FIN was merely
queued from one the peer actually received. That matters before a reset:
RESET_STREAM on a stream still in DataSent discards the unacknowledged
data, so a final message can be dropped even though stream_send()
reported it written. Callers that want a delivery guarantee currently
have to avoid resetting streams entirely, or invent an application-level
acknowledgement.

Report is_shutdown()/is_stopped() streams as incomplete rather than
deferring to is_complete(). A reset counts as complete for collection
purposes, since there is nothing left to send either way, but as an
answer to "did the peer get my FIN" it would be wrong.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@kixelated
kixelated requested a review from a team as a code owner August 6, 2026 01:08

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 9a3c88f90f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread quiche/src/lib.rs
return false;
}

stream.send.is_complete()

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Track FIN acknowledgement separately from byte acknowledgements

When an application sends data without FIN, waits for those bytes to be acknowledged, and then closes the stream with an empty stream_send(..., true), setting fin_off makes the already-complete byte range satisfy SendBuf::is_complete() immediately—even before the FIN is emitted or acknowledged. This false positive lets a caller reset the stream and discard the FIN despite this API promising that the peer acknowledged it; FIN acknowledgement needs separate state rather than relying solely on the acknowledged byte range.

Useful? React with 👍 / 👎.

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

Labels

None yet

1 participant