Skip to content

Conversation

@George-Miao
Copy link
Member

No description provided.

Copilot AI review requested due to automatic review settings January 28, 2026 16:25
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refactors the driver OpCode abstraction to be unsafe, making the safety invariants explicit at the trait boundary (polling / io-uring / IOCP).

Changes:

  • Converts OpCode traits in poll, io-uring, and IOCP backends to pub unsafe trait OpCode.
  • Updates all affected OpCode implementations across crates/tests to unsafe impl OpCode ....
  • Adjusts trait docs to introduce # Safety sections describing (some of) the required invariants.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
compio-runtime/tests/event.rs Updates test OpCode impl to unsafe impl for Windows event waiting.
compio-process/src/windows.rs Marks WaitProcess as unsafe impl OpCode for IOCP integration.
compio-net/src/poll_fd/windows.rs Marks WaitWSAEvent as unsafe impl OpCode.
compio-fs/src/stdio/windows.rs Marks StdRead/StdWrite as unsafe impl OpCode (blocking ops).
compio-driver/tests/splice.rs Updates Splice test op to unsafe impl OpCode.
compio-driver/src/sys/poll/op.rs Converts poll backend operation impls to unsafe impl OpCode.
compio-driver/src/sys/poll/mod.rs Makes poll backend OpCode trait unsafe and updates docs section heading to # Safety.
compio-driver/src/sys/iour/op.rs Converts io-uring backend operation impls to unsafe impl OpCode.
compio-driver/src/sys/iour/mod.rs Makes io-uring backend OpCode trait unsafe and adds a safety note.
compio-driver/src/sys/iocp/op.rs Converts IOCP backend operation impls to unsafe impl OpCode.
compio-driver/src/sys/iocp/mod.rs Makes IOCP backend OpCode trait unsafe and adds a safety note.
compio-driver/src/sys/fusion/op.rs Updates fusion wrappers to unsafe impl for poll/io-uring OpCode forwarding.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +113 to +114
/// The returned Entry from `create_entry` must be valid until the operation is
/// completed.
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

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

The new # Safety contract for this unsafe trait is too ambiguous: the io_uring::squeue::Entry value itself does not need to stay alive, but any resources it refers to (e.g., buffer pointers/lengths, iovec arrays, strings, etc.) must remain valid (and properly aligned) until the operation completes or is cancelled. Please tighten the wording to explicitly state what must be kept alive and what constitutes “completed” (incl. cancellation).

Suggested change
/// The returned Entry from `create_entry` must be valid until the operation is
/// completed.
/// Implementors must uphold the safety contract between the io-uring submission
/// entry created by [`create_entry`] and any user-space resources it refers to.
///
/// In particular:
/// - The `io_uring::squeue::Entry` / `Entry128` value itself does *not* need to
/// stay alive after `create_entry` returns; the runtime may move, copy, or drop
/// that value at will.
/// - However, every resource that the submission entry *refers to* (for example,
/// buffer pointers and lengths, `iovec`/slice arrays, strings, and any other
/// memory or file descriptors passed to the kernel through the SQE) must remain
/// allocated, properly aligned, and valid for the kernel to read/write for the
/// entire lifetime of the in-flight operation.
///
/// For the purposes of this contract, an operation is considered *completed* once
/// the runtime has finished handling the associated io-uring completion queue
/// entry (CQE), including CQEs that report an explicit cancellation (for example,
/// via an `AsyncCancel` request). Until that point, implementations must not free,
/// move, or otherwise invalidate any memory or descriptors that are referenced by
/// the submission entry in a way that would make the kernel observe dangling or
/// misaligned pointers.

Copilot uses AI. Check for mistakes.
Comment on lines 276 to +282
/// Abstraction of IOCP operations.
pub trait OpCode {
///
/// # Safety
///
/// Implementors must ensure that the operation is safe to be polled
/// according to the returned [`OpType`].
pub unsafe trait OpCode {
Copy link

Copilot AI Jan 28, 2026

Choose a reason for hiding this comment

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

Since OpCode is now an unsafe trait, the # Safety section should enumerate the concrete invariants implementors must uphold (e.g., OpType::Event handles must remain valid until completion, OpType::Blocking requires operate to be thread-safe, and Overlapped requires correct use of optr). Right now it’s very high-level and easy for downstream implementors to miss required guarantees that can lead to UB.

Copilot uses AI. Check for mistakes.
@George-Miao George-Miao merged commit 35827eb into compio-rs:master Jan 28, 2026
60 checks passed
@George-Miao George-Miao deleted the refactor/driver/unsafe-op-trait branch January 28, 2026 16:37
@github-actions github-actions bot mentioned this pull request Jan 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking change package: driver Related to compio-driver

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants