refactor(driver): record multishot results in ops#748
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR refactors io-uring multishot handling so that per-shot results (and selected managed buffers) are recorded inside the operation itself, addressing the buffer-loss scenario described in #746 when a managed op completes after cancellation.
Changes:
- Move multishot result storage from the io-uring driver (
HashMap<ErasedKey, VecDeque<_>>) into the op implementations via newOpCode::{push_multishot,pop_multishot}hooks. - Extend io-uring
OpCode::set_resultto receive&io::Result<usize>and&Extra, allowing ops (notably managed-buffer ops) to record/own selected buffers for later cleanup. - Refactor io-uring buffer pool internals to be
Clone/Rc-backed and introduce an owned buffer handle that returns buffers on drop.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| compio-driver/src/sys/stub/mod.rs | Removes no-longer-needed cleanup_multishot stub API. |
| compio-driver/src/sys/poll/mod.rs | Removes no-longer-needed cleanup_multishot for polling driver. |
| compio-driver/src/sys/iour/op.rs | Implements per-op multishot queues and records selected buffers in managed ops. |
| compio-driver/src/sys/iour/mod.rs | Routes multishot CQEs directly into ops; removes driver-level multishot storage; adjusts buffer-pool release. |
| compio-driver/src/sys/iocp/mod.rs | Removes no-longer-needed cleanup_multishot for IOCP driver. |
| compio-driver/src/sys/fusion/op.rs | For fused ops, forwards the expanded io-uring OpCode surface (fallback/blocking/result/multishot). |
| compio-driver/src/sys/fusion/mod.rs | Removes driver-level cleanup_multishot forwarding. |
| compio-driver/src/op.rs | Renames managed fallback buffer type to FallbackOwnedBuffer to disambiguate with io-uring owned buffers. |
| compio-driver/src/lib.rs | Removes cleanup_multishot calls during pop/pop_with_extra since storage moved into ops. |
| compio-driver/src/key.rs | Calls OpCode::set_result(&Result, &Extra) for io-uring completions to let ops capture selected resources. |
| compio-driver/src/buffer_pool/iour.rs | Makes io-uring buffer pool Clone (Rc-backed) and introduces an owned buffer handle with drop-based reuse. |
| compio-driver/src/buffer_pool/fusion.rs | Renames/re-exports owned buffer types (FallbackOwnedBuffer, IoUringOwnedBuffer) for fusion builds. |
| compio-driver/src/buffer_pool/fallback.rs | Exposes FallbackOwnedBuffer re-export for non-fusion builds. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
George-Miao
reviewed
Mar 9, 2026
George-Miao
reviewed
Mar 9, 2026
George-Miao
requested changes
Mar 9, 2026
Member
|
With BufferPools internalized into Ops now, does that mean maybe we can refactored Managed api and return buffers directly from returned Ops? i.e., |
09c6300 to
bb51df2
Compare
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.
Closes #746