Skip to content

7992: io: AioSource now employs IO Safety #85

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
martin-augment wants to merge 1 commit into
base: master
Choose a base branch
from
Open

7992: io: AioSource now employs IO Safety #85

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions tokio/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ pin-project-lite = "0.2.11"

# Everything else is optional...
bytes = { version = "1.2.1", optional = true }
mio = { version = "1.0.1", optional = true, default-features = false }
mio = { version = "1.2.0", optional = true, default-features = false }
Copy link
Copy Markdown

@augmentcode augmentcode bot Mar 29, 2026

Choose a reason for hiding this comment

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

These dependency bumps (mio/nix/mio-aio) could implicitly raise the effective MSRV above rust-version = "1.71"; consider ensuring CI still exercises/builds on 1.71 so this doesn’t regress older toolchains.

Severity: medium

Other Locations
  • tokio/Cargo.toml:115
  • tokio/Cargo.toml:125
  • tokio/Cargo.toml:160

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

Copy link
Copy Markdown
Owner Author

@martin-augment martin-augment Mar 29, 2026

Choose a reason for hiding this comment

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

value:good-to-have; category:bug; feedback: The Augment AI reviewer is correct! The Minimum Supported Rust Version (MSRV) could change while updating dependencies. The Tokio project has a CI job that checks the build with 1.71, so such problems would be caught early!

parking_lot = { version = "0.12.0", optional = true }

[target.'cfg(not(target_family = "wasm"))'.dependencies]
Expand All @@ -112,7 +112,7 @@ tracing = { version = "0.1.29", default-features = false, features = ["std"], op
[target.'cfg(all(tokio_unstable, target_os = "linux"))'.dependencies]
io-uring = { version = "0.7.11", default-features = false, optional = true }
libc = { version = "0.2.168", optional = true }
mio = { version = "1.0.1", default-features = false, features = ["os-poll", "os-ext"], optional = true }
mio = { version = "1.2.0", default-features = false, features = ["os-poll", "os-ext"], optional = true }
slab = { version = "0.4.9", optional = true }
backtrace = { version = "0.3.58", optional = true }

Expand All @@ -122,7 +122,7 @@ signal-hook-registry = { version = "1.1.1", optional = true }

[target.'cfg(unix)'.dev-dependencies]
libc = { version = "0.2.168" }
nix = { version = "0.29.0", default-features = false, features = ["aio", "fs", "socket"] }
nix = { version = "0.31.0", default-features = false, features = ["aio", "fs", "socket"] }

[target.'cfg(windows)'.dependencies.windows-sys]
version = "0.61"
Expand Down Expand Up @@ -157,7 +157,7 @@ rand = "0.9"
wasm-bindgen-test = "0.3.0"

[target.'cfg(target_os = "freebsd")'.dev-dependencies]
mio-aio = { version = "1", features = ["tokio"] }
mio-aio = { version = "2", features = ["tokio"] }

[target.'cfg(loom)'.dev-dependencies]
loom = { version = "0.7", features = ["futures", "checkpoint"] }
Expand Down
9 changes: 4 additions & 5 deletions tokio/src/io/bsd/poll_aio.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,7 @@ use mio::Token;
use std::fmt;
use std::io;
use std::ops::{Deref, DerefMut};
use std::os::unix::io::AsRawFd;
use std::os::unix::prelude::RawFd;
use std::os::fd::{AsFd, BorrowedFd};
use std::task::{ready, Context, Poll};

/// Like [`mio::event::Source`], but for POSIX AIO only.
Expand All @@ -19,7 +18,7 @@ use std::task::{ready, Context, Poll};
/// [`Aio`] object.
pub trait AioSource {
/// Registers this AIO event source with Tokio's reactor.
fn register(&mut self, kq: RawFd, token: usize);
fn register(&mut self, kq: BorrowedFd<'_>, token: usize);
Copy link
Copy Markdown

@augmentcode augmentcode bot Mar 29, 2026

Choose a reason for hiding this comment

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

tokio::io::bsd::AioSource is publicly re-exported, so changing register from RawFd to BorrowedFd is a breaking API change for FreeBSD users; can you confirm this is acceptable for the release/changelog you’re targeting?

Severity: medium

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎, or 🚀 if it prevented an incident/outage.

Copy link
Copy Markdown
Owner Author

@martin-augment martin-augment Mar 29, 2026

Choose a reason for hiding this comment

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

value:useful; category:bug; feedback: The Augment AI reviewer is correct! The signature change is an API breakage and it cannot be made. A new method will need to be added to the trait with a default implementation, so that it does not break the existing implementations .


Comment on lines 19 to 22
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 29, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

This is a public API break for AioSource implementers.

Changing register from raw-fd-style input to BorrowedFd<'_> will break downstream impls at compile time. If intentional, this should be treated as a breaking-change path (or have a compatibility strategy / explicit release-note callout).

🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@tokio/src/io/bsd/poll_aio.rs` around lines 19 - 22, The change to
AioSource::register to take BorrowedFd<'_> breaks existing implementers; restore
a non-breaking API by reintroducing the original signature register(&mut self,
fd: RawFd, token: usize) (or provide a default method with that signature) and
implement a small wrapper that converts the RawFd into a BorrowedFd and calls
the new BorrowedFd-based implementation (or vice versa) so existing impls
compile; update the trait to include both signatures (one delegating to the
other) and ensure the public trait symbol AioSource::register remains usable by
downstream code.

Copy link
Copy Markdown
Owner Author

@martin-augment martin-augment Mar 29, 2026

Choose a reason for hiding this comment

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

value:useful; category:bug; feedback: The CodeRabbit AI reviewer is correct! The signature change is an API breakage and it cannot be made. A new method will need to be added to the trait with a default implementation, so that it does not break the implementers .

/// Deregisters this AIO event source with Tokio's reactor.
fn deregister(&mut self);
Expand All @@ -37,7 +36,7 @@ impl<T: AioSource> Source for MioSource<T> {
interests: mio::Interest,
) -> io::Result<()> {
assert!(interests.is_aio() || interests.is_lio());
self.0.register(registry.as_raw_fd(), usize::from(token));
self.0.register(registry.as_fd(), usize::from(token));
Ok(())
}

Expand All @@ -53,7 +52,7 @@ impl<T: AioSource> Source for MioSource<T> {
interests: mio::Interest,
) -> io::Result<()> {
assert!(interests.is_aio() || interests.is_lio());
self.0.register(registry.as_raw_fd(), usize::from(token));
self.0.register(registry.as_fd(), usize::from(token));
Ok(())
}
}
Expand Down
9 changes: 5 additions & 4 deletions tokio/tests/io_async_fd.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ use std::time::Duration;
use std::{
future::Future,
io::{self, ErrorKind, Read, Write},
os::fd::OwnedFd,
task::{Context, Waker},
};

Expand Down Expand Up @@ -69,7 +70,7 @@ impl AsRawFd for FileDescriptor {

impl Read for &FileDescriptor {
fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
read(self.fd.as_raw_fd(), buf).map_err(io::Error::from)
read(&self.fd, buf).map_err(io::Error::from)
}
}

Expand Down Expand Up @@ -99,7 +100,7 @@ impl Write for FileDescriptor {
}
}

fn set_nonblocking(fd: RawFd) {
fn set_nonblocking(fd: &OwnedFd) {
use nix::fcntl::{OFlag, F_GETFL, F_SETFL};

let flags = nix::fcntl::fcntl(fd, F_GETFL).expect("fcntl(F_GETFD)");
Expand Down Expand Up @@ -129,8 +130,8 @@ fn socketpair() -> (FileDescriptor, FileDescriptor) {
.expect("socketpair");
let fds = (FileDescriptor { fd: fd_a }, FileDescriptor { fd: fd_b });

set_nonblocking(fds.0.fd.as_raw_fd());
set_nonblocking(fds.1.fd.as_raw_fd());
set_nonblocking(&fds.0.fd);
set_nonblocking(&fds.1.fd);

fds
}
Expand Down
9 changes: 5 additions & 4 deletions tokio/tests/io_async_fd_memory_leak.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ fn allocated_bytes() -> usize {
#[tokio::test]
async fn memory_leak_when_fd_closed_before_drop() {
use nix::sys::socket::{self, AddressFamily, SockFlag, SockType};
use std::os::fd::OwnedFd;
use std::os::unix::io::{AsRawFd, RawFd};
use std::sync::Arc;
use tokio::io::unix::AsyncFd;
Expand All @@ -83,7 +84,7 @@ async fn memory_leak_when_fd_closed_before_drop() {
}
}

fn set_nonblocking(fd: RawFd) {
fn set_nonblocking(fd: &OwnedFd) {
use nix::fcntl::{OFlag, F_GETFL, F_SETFL};

let flags = nix::fcntl::fcntl(fd, F_GETFL).expect("fcntl(F_GETFL)");
Expand Down Expand Up @@ -119,7 +120,7 @@ async fn memory_leak_when_fd_closed_before_drop() {
.unwrap();

let raw_fd = fd_a.as_raw_fd();
set_nonblocking(raw_fd);
set_nonblocking(&fd_a);
std::mem::forget(fd_a);

let wrapper = Arc::new(RawFdWrapper { fd: raw_fd });
Expand Down Expand Up @@ -148,7 +149,7 @@ async fn memory_leak_when_fd_closed_before_drop() {
.unwrap();

let raw_fd = fd_a.as_raw_fd();
set_nonblocking(raw_fd);
set_nonblocking(&fd_a);
std::mem::forget(fd_a);

let wrapper = Arc::new(RawFdWrapper { fd: raw_fd });
Expand All @@ -175,7 +176,7 @@ async fn memory_leak_when_fd_closed_before_drop() {
.unwrap();

let raw_fd = fd_a.as_raw_fd();
set_nonblocking(raw_fd);
set_nonblocking(&fd_a);
std::mem::forget(fd_a);

let wrapper = Arc::new(RawFdWrapper { fd: raw_fd });
Expand Down
14 changes: 7 additions & 7 deletions tokio/tests/io_poll_aio.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,8 @@ use mio_aio::{AioFsyncMode, SourceApi};
use std::{
future::Future,
io, mem,
os::fd::AsFd,
os::unix::io::{AsRawFd, RawFd},
os::fd::{AsFd, BorrowedFd},
os::unix::io::AsRawFd,
pin::{pin, Pin},
task::{Context, Poll},
};
Expand All @@ -21,7 +21,7 @@ mod aio {
struct TokioSource<'fd>(mio_aio::Source<nix::sys::aio::AioFsync<'fd>>);

impl<'fd> AioSource for TokioSource<'fd> {
fn register(&mut self, kq: RawFd, token: usize) {
fn register(&mut self, kq: BorrowedFd<'_>, token: usize) {
self.0.register_raw(kq, token)
}
fn deregister(&mut self) {
Expand Down Expand Up @@ -81,10 +81,10 @@ mod aio {
}

impl AioSource for LlSource {
fn register(&mut self, kq: RawFd, token: usize) {
fn register(&mut self, kq: BorrowedFd<'_>, token: usize) {
let mut sev: libc::sigevent = unsafe { mem::MaybeUninit::zeroed().assume_init() };
sev.sigev_notify = libc::SIGEV_KEVENT;
sev.sigev_signo = kq;
sev.sigev_signo = kq.as_raw_fd();
sev.sigev_value = libc::sigval {
sival_ptr: token as *mut libc::c_void,
};
Expand Down Expand Up @@ -222,10 +222,10 @@ mod lio {
}

impl<'a> AioSource for LioSource<'a> {
fn register(&mut self, kq: RawFd, token: usize) {
fn register(&mut self, kq: BorrowedFd<'_>, token: usize) {
let mut sev: libc::sigevent = unsafe { mem::MaybeUninit::zeroed().assume_init() };
sev.sigev_notify = libc::SIGEV_KEVENT;
sev.sigev_signo = kq;
sev.sigev_signo = kq.as_raw_fd();
sev.sigev_value = libc::sigval {
sival_ptr: token as *mut libc::c_void,
};
Expand Down
6 changes: 3 additions & 3 deletions tokio/tests/net_unix_pipe.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ use tokio_test::{assert_err, assert_ok, assert_pending, assert_ready_ok};

use std::fs::File;
use std::io;
use std::os::fd::AsFd;
use std::os::unix::fs::OpenOptionsExt;
use std::os::unix::io::AsRawFd;
use std::path::{Path, PathBuf};

/// Helper struct which will clean up temporary files once dropped.
Expand Down Expand Up @@ -277,8 +277,8 @@ async fn from_file_detects_wrong_access_mode() -> io::Result<()> {
Ok(())
}

fn is_nonblocking<T: AsRawFd>(fd: &T) -> io::Result<bool> {
let flags = nix::fcntl::fcntl(fd.as_raw_fd(), nix::fcntl::F_GETFL)?;
fn is_nonblocking<T: AsFd>(fd: &T) -> io::Result<bool> {
let flags = nix::fcntl::fcntl(fd.as_fd(), nix::fcntl::F_GETFL)?;
Ok((flags & libc::O_NONBLOCK) != 0)
}

Expand Down
Loading