embedded-io-async: clarify cancel safety semantics for Read and Write#734
Closed
aki1770-del wants to merge 1 commit intorust-embedded:masterfrom
Closed
embedded-io-async: clarify cancel safety semantics for Read and Write#734aki1770-del wants to merge 1 commit intorust-embedded:masterfrom
aki1770-del wants to merge 1 commit intorust-embedded:masterfrom
Conversation
…dded#719) The previous documentation used permissive language ("encouraged", "implementations should document") that left callers uncertain about what happens to data in flight when a future is dropped mid-transfer. Replace the vague encouragement with normative documentation that: - States explicitly the method is NOT cancel-safe by default - Describes the concrete failure mode: bytes already received from hardware FIFO/DMA may be silently discarded with no error returned - Uses SHOULD/MUST language consistent with the rest of the trait docs - Mentions the CancelSafeRead marker trait path for future opt-in - Gives callers actionable guidance (run future to completion) This follows the pattern established by Tokio's cancel-safety documentation and PR rust-embedded#469 ("io: expand docs"), closing the gap that issue rust-embedded#719 identified. Fixes rust-embedded#719 Signed-off-by: aki1770-del <aki1770@gmail.com>
Member
|
As far as I can tell the old and new wording is equivalent (implementations aren't required to be cancel-safe, implementations should document whether they are cancel-safe or not). So this PR is just a reword, it doesn't "fix" anything. I personally find RFC-style MUST, SHOULD, MAY somewhat obnoxious to read, and it feels out of place here since we don't use it in the rest of rustdocs. |
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
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.
Fixes #719
Problem
The documentation for
Read::readandWrite::writeinembedded-io-asyncused permissive language — "implementations are encouraged" and "implementations should document" — that left both callers and implementors without a clear contract. Issue #719 identifies the concrete failure mode this ambiguity enables:When a
read()future is dropped mid-transfer, bytes already received from the hardware FIFO or DMA buffer are silently discarded. No error is returned. The caller has no way to detect or recover from the data loss. In safety-critical embedded contexts (CAN/UART frames, ISO 26262), this is a silent data loss that can propagate through state machines without triggering any diagnostic path.Fix
Rewrite the cancel-safety sections using normative
SHOULD/MUSTlanguage and a dedicated# Cancel Safetyheading (consistent with Tokio's convention):CancelSafeReadmarker trait path for opt-inThe PR changes documentation only. No trait methods are added, no signatures change, no semver bump required. The architectural path to a
CancelSafeReadmarker trait (issue #719's long-term ask) is left for a follow-up RFC.Existing Behavior Audit (OPS-RULE-016)
embedded-io-async/src/lib.rsRead::readcancel-safety: "encouraged", "should document" — no normative contractembedded-io-async/src/lib.rsWrite::writecancel-safety: identical permissive framingScope Classification (OPS-RULE-017)
Category (b): Minor enhancement (documentation). No trait method signatures changed. No API-breaking change. A normative documentation tightening for existing behavior that was already the intended semantic, per the issue discussion. A full API change (new
CancelSafeReadtrait) would be category (c) requiring RFC — that is out of scope here.Test
cargo check -p embedded-io-asyncpasses. Documentation-only changes require no additional test code; the trait method signatures are unchanged.Prior Art (OPS-RULE-018)
&'static mutreferences #37 — "Explore DMA-based APIs built on top of&'static mutreferences" (open, 2018) — root structural reason cancel safety is non-trivial for hardware FIFO implementations; the DMA buffer ownership problem this fix warns aboutAI-assisted — authored with Claude, reviewed by Komada.