Skip to content
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.

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

Implement I/O-safe traits on types #1036

Merged
merged 6 commits into from
May 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 15 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,21 @@ jobs:
with:
command: check
args: --all --features tokio02

check_io_safety_feature:
name: Check io_safety feature
runs-on: ${{ matrix.os }}
strategy:
matrix:
os: [ubuntu-latest, windows-latest]
Copy link
Member

Choose a reason for hiding this comment

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

Doesn’t support MacOS?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The changes I wrote only impact cfg(unix) and cfg(windows), so I included a Unix platform (ubuntu-latest) and a Windows platform (windows-latest). If you would like me to also test on MacOS I can.

steps:
- uses: actions/checkout@v3
- name: check io_safety
uses: actions-rs/cargo@v1
with:
command: check
args: --all --features io_safety


cross:
name: Cross compile
Expand Down
1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ alloc = [
tokio1 = ["async-global-executor/tokio"]
tokio02 = ["async-global-executor/tokio02"]
tokio03 = ["async-global-executor/tokio03"]
io_safety = []

[dependencies]
async-attributes = { version = "1.1.1", optional = true }
Expand Down
67 changes: 61 additions & 6 deletions src/fs/file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ use crate::prelude::*;
use crate::task::{spawn_blocking, Context, Poll, Waker};
use crate::utils::Context as _;

const ARC_TRY_UNWRAP_EXPECT: &str = "cannot acquire ownership of the file handle after drop";

/// An open file on the filesystem.
///
/// Depending on what options the file was opened with, this type can be used for reading and/or
Expand Down Expand Up @@ -415,6 +417,15 @@ impl From<std::fs::File> for File {
cfg_unix! {
use crate::os::unix::io::{AsRawFd, FromRawFd, IntoRawFd, RawFd};

impl File {
fn into_std_file(self) -> std::fs::File {
let file = self.file.clone();
drop(self);
Arc::try_unwrap(file)
.expect(ARC_TRY_UNWRAP_EXPECT)
}
}

impl AsRawFd for File {
fn as_raw_fd(&self) -> RawFd {
self.file.as_raw_fd()
Expand All @@ -429,11 +440,29 @@ cfg_unix! {

impl IntoRawFd for File {
fn into_raw_fd(self) -> RawFd {
let file = self.file.clone();
drop(self);
Arc::try_unwrap(file)
.expect("cannot acquire ownership of the file handle after drop")
.into_raw_fd()
self.into_std_file().into_raw_fd()
}
}

cfg_io_safety! {
use crate::os::unix::io::{AsFd, BorrowedFd, OwnedFd};

impl AsFd for File {
fn as_fd(&self) -> BorrowedFd<'_> {
self.file.as_fd()
}
}

impl From<OwnedFd> for File {
fn from(fd: OwnedFd) -> Self {
std::fs::File::from(fd).into()
}
}

impl From<File> for OwnedFd {
Copy link
Member

Choose a reason for hiding this comment

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

Should this be TryFrom?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was mirroring the previous implementation for IntoRawFd for this type. If you would think that it would be better as TryFrom I will oblige.

fn from(val: File) -> OwnedFd {
self.into_std_file().into()
}
}
}
}
Expand All @@ -458,10 +487,36 @@ cfg_windows! {
let file = self.file.clone();
drop(self);
Arc::try_unwrap(file)
.expect("cannot acquire ownership of the file handle after drop")
.expect(ARC_TRY_UNWRAP_EXPECT)
.into_raw_handle()
}
}

cfg_io_safety! {
use crate::os::windows::io::{AsHandle, BorrowedHandle, OwnedHandle};

impl AsHandle for File {
fn as_handle(&self) -> BorrowedHandle<'_> {
self.file.as_handle()
}
}

impl From<OwnedHandle> for File {
fn from(handle: OwnedHandle) -> Self {
std::fs::File::from(handle).into()
}
}

impl From<File> for OwnedHandle {
fn from(val: File) -> OwnedHandle {
let file = val.file.clone();
drop(val);
Arc::try_unwrap(file)
.expect(ARC_TRY_UNWRAP_EXPECT)
.into()
}
}
}
}

/// An async mutex with non-borrowing lock guards.
Expand Down
20 changes: 20 additions & 0 deletions src/io/stderr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,16 @@ cfg_unix! {
std::io::stderr().as_raw_fd()
}
}

cfg_io_safety! {
use crate::os::unix::io::{AsFd, BorrowedFd};

impl AsFd for Stderr {
fn as_fd(&self) -> BorrowedFd<'_> {
std::io::stderr().as_fd()
}
}
}
}

cfg_windows! {
Expand All @@ -190,4 +200,14 @@ cfg_windows! {
std::io::stderr().as_raw_handle()
}
}

cfg_io_safety! {
use crate::os::unix::io::{AsHandle, BorrowedHandle};

impl AsHandle for Stderr {
fn as_handle(&self) -> BorrowedHandle<'_> {
std::io::stderr().as_handle()
}
}
}
}
20 changes: 20 additions & 0 deletions src/io/stdin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,16 @@ cfg_unix! {
std::io::stdin().as_raw_fd()
}
}

cfg_io_safety! {
use crate::os::unix::io::{AsFd, BorrowedFd};

impl AsFd for Stderr {
fn as_fd(&self) -> BorrowedFd<'_> {
std::io::stdin().as_fd()
}
}
}
}

cfg_windows! {
Expand All @@ -216,4 +226,14 @@ cfg_windows! {
std::io::stdin().as_raw_handle()
}
}

cfg_io_safety! {
use crate::os::unix::io::{AsFd, BorrowedFd};

impl AsFd for Stdin {
fn as_fd(&self) -> BorrowedFd<'_> {
std::io::stdin().as_fd()
}
}
}
}
20 changes: 20 additions & 0 deletions src/io/stdout.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,16 @@ cfg_unix! {
std::io::stdout().as_raw_fd()
}
}

cfg_io_safety! {
use crate::os::unix::io::{AsFd, BorrowedFd};

impl AsFd for Stdout {
fn as_fd(&self) -> BorrowedFd<'_> {
std::io::stdout().as_fd()
}
}
}
}

cfg_windows! {
Expand All @@ -190,4 +200,14 @@ cfg_windows! {
std::io::stdout().as_raw_handle()
}
}

cfg_io_safety! {
use crate::os::unix::io::{AsHandle, BorrowedHandle};

impl AsHandle for Stdout {
fn as_handle(&self) -> BorrowedHandle<'_> {
std::io::stdout().as_handle()
}
}
}
}
44 changes: 44 additions & 0 deletions src/net/tcp/listener.rs
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,28 @@ cfg_unix! {
self.watcher.into_inner().unwrap().into_raw_fd()
}
}

cfg_io_safety! {
use crate::os::unix::io::{AsFd, BorrowedFd, OwnedFd};

impl AsFd for TcpListener {
fn as_fd(&self) -> BorrowedFd<'_> {
self.watcher.get_ref().as_fd()
}
}

impl From<OwnedFd> for TcpListener {
fn from(fd: OwnedFd) -> TcpListener {
std::net::TcpListener::from(fd).into()
}
}

impl From<TcpListener> for OwnedFd {
fn from(listener: TcpListener) -> OwnedFd {
listener.watcher.into_inner().unwrap().into()
}
}
}
}

cfg_windows! {
Expand All @@ -306,4 +328,26 @@ cfg_windows! {
self.watcher.into_inner().unwrap().into_raw_socket()
}
}

cfg_io_safety! {
use crate::os::windows::io::{AsSocket, BorrowedSocket, OwnedSocket};

impl AsSocket for TcpListener {
fn as_socket(&self) -> BorrowedSocket<'_> {
self.watcher.get_ref().as_socket()
}
}

impl From<OwnedSocket> for TcpListener {
fn from(fd: OwnedSocket) -> TcpListener {
std::net::TcpListener::from(fd).into()
}
}

impl From<TcpListener> for OwnedSocket {
fn from(listener: TcpListener) -> OwnedSocket {
listener.watcher.into_inner().unwrap().into()
Copy link
Member

Choose a reason for hiding this comment

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

Should this be TryFrom?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was mirroring the IntoRawSocket implementation for this type. Should this be TryFrom? If so, I can make it.

}
}
}
}
44 changes: 44 additions & 0 deletions src/net/tcp/stream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -416,6 +416,28 @@ cfg_unix! {
self.as_raw_fd()
}
}

cfg_io_safety! {
use crate::os::unix::io::{AsFd, BorrowedFd, OwnedFd};

impl AsFd for TcpStream {
fn as_fd(&self) -> BorrowedFd<'_> {
self.watcher.get_ref().as_fd()
}
}

impl From<OwnedFd> for TcpStream {
fn from(fd: OwnedFd) -> TcpStream {
std::net::TcpStream::from(fd).into()
}
}

impl From<TcpStream> for OwnedFd {
fn from(stream: TcpStream) -> OwnedFd {
stream.watcher.into_inner().unwrap().into()
}
}
}
}

cfg_windows! {
Expand Down Expand Up @@ -443,4 +465,26 @@ cfg_windows! {
self.as_raw_socket()
}
}

cfg_io_safety! {
use crate::os::windows::io::{AsSocket, BorrowedSocket, OwnedSocket};

impl AsSocket for TcpStream {
fn as_socket(&self) -> BorrowedSocket<'_> {
self.watcher.get_ref().as_socket()
}
}

impl From<OwnedSocket> for TcpStream {
fn from(fd: OwnedSocket) -> TcpStream {
std::net::TcpListener::from(fd).into()
}
}

impl From<TcpStream> for OwnedSocket {
fn from(stream: TcpStream) -> OwnedSocket {
stream.watcher.into_inner().unwrap().into()
}
}
}
}
44 changes: 44 additions & 0 deletions src/net/udp/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -562,6 +562,28 @@ cfg_unix! {
self.watcher.into_inner().unwrap().into_raw_fd()
}
}

cfg_io_safety! {
use crate::os::unix::io::{AsFd, BorrowedFd, OwnedFd};

impl AsFd for UdpSocket {
fn as_fd(&self) -> BorrowedFd<'_> {
self.watcher.get_ref().as_fd()
}
}

impl From<OwnedFd> for UdpSocket {
fn from(fd: OwnedFd) -> UdpSocket {
std::net::TcpStream::from(fd).into()
}
}

impl From<UdpSocket> for OwnedFd {
fn from(stream: UdpSocket) -> OwnedFd {
stream.watcher.into_inner().unwrap().into()
}
}
}
}

cfg_windows! {
Expand All @@ -586,4 +608,26 @@ cfg_windows! {
self.watcher.into_inner().unwrap().into_raw_socket()
}
}

cfg_io_safety! {
use crate::os::windows::io::{AsSocket, BorrowedSocket, OwnedSocket};

impl AsSocket for UdpSocket {
fn as_socket(&self) -> BorrowedSocket<'_> {
self.watcher.get_ref().as_socket()
}
}

impl From<OwnedSocket> for UdpSocket {
fn from(fd: OwnedSocket) -> UdpSocket {
std::net::TcpListener::from(fd).into()
}
}

impl From<UdpSocket> for OwnedSocket {
fn from(stream: UdpSocket) -> OwnedSocket {
stream.watcher.into_inner().unwrap().into()
}
}
}
}