Add Connection::stream_send_complete() - #2571
Conversation
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>
There was a problem hiding this comment.
💡 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".
| return false; | ||
| } | ||
|
|
||
| stream.send.is_complete() |
There was a problem hiding this comment.
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 👍 / 👎.
(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