From aa5e26bc7fd7e95084c64538ca1389d3cc06c3c4 Mon Sep 17 00:00:00 2001 From: Ivan Gankevich Date: Fri, 27 Dec 2024 09:36:38 +0100 Subject: [PATCH 01/27] Implement process polling via `pidfd`. --- Cargo.toml | 2 + src/lib.rs | 4 ++ src/process.rs | 116 ++++++++++++++++++++++++++++++++++ src/sys/unix/mod.rs | 13 ++++ src/sys/unix/process/pidfd.rs | 74 ++++++++++++++++++++++ tests/process.rs | 77 ++++++++++++++++++++++ tests/util/mod.rs | 22 +++++-- 7 files changed, 303 insertions(+), 5 deletions(-) create mode 100644 src/process.rs create mode 100644 src/sys/unix/process/pidfd.rs create mode 100644 tests/process.rs diff --git a/Cargo.toml b/Cargo.toml index b280287b4..096ace739 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -42,6 +42,8 @@ os-ext = [ ] # Enables `mio::net` module containing networking primitives. net = [] +# Enables OS process polling. +process = [] [dependencies] log = { version = "0.4.8", optional = true } diff --git a/src/lib.rs b/src/lib.rs index 8004ee2bb..96400a7da 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -46,6 +46,8 @@ mod macros; mod interest; mod poll; +#[cfg(feature = "process")] +mod process; mod sys; mod token; #[cfg(not(target_os = "wasi"))] @@ -68,6 +70,8 @@ pub use poll::{Poll, Registry}; pub use token::Token; #[cfg(not(target_os = "wasi"))] pub use waker::Waker; +#[cfg(feature = "process")] +pub use process::Process; #[cfg(all(unix, feature = "os-ext"))] #[cfg_attr(docsrs, doc(cfg(all(unix, feature = "os-ext"))))] diff --git a/src/process.rs b/src/process.rs new file mode 100644 index 000000000..1d0c76f54 --- /dev/null +++ b/src/process.rs @@ -0,0 +1,116 @@ +use std::io::Error; +#[cfg(any( + target_os = "android", + target_os = "espidf", + target_os = "fuchsia", + target_os = "hermit", + target_os = "illumos", + target_os = "linux", +))] +use std::os::fd::{AsRawFd, FromRawFd, IntoRawFd, RawFd}; +use std::process::Child; + +use crate::event::Source; +use crate::{sys, Interest, Registry, Token}; + +/// Process allows polling OS processes for completion. +/// +/// When the process exits the event with [`readable`] readiness is generated. +/// +/// # Notes +/// +/// Events are delivered even if the process has exited by the time [`poll`](crate::Poll::poll) is +/// called. +/// +/// # Implementation notes +/// +/// On Linux `Process` uses `pidfd`. +#[derive(Debug)] +pub struct Process { + inner: sys::Process, +} + +impl Process { + /// Create new process from [`Child`](std::process::Child). + pub fn new(child: &Child) -> Result { + let inner = sys::Process::new(child)?; + Ok(Self { inner }) + } + + /// Create new process from the process id. + #[cfg(unix)] + pub fn from_pid(pid: libc::pid_t) -> Result { + let inner = sys::Process::from_pid(pid)?; + Ok(Self { inner }) + } +} + +impl Source for Process { + fn register( + &mut self, + registry: &Registry, + token: Token, + interests: Interest, + ) -> Result<(), Error> { + self.inner.register(registry, token, interests) + } + + fn reregister( + &mut self, + registry: &Registry, + token: Token, + interests: Interest, + ) -> Result<(), Error> { + self.inner.reregister(registry, token, interests) + } + + fn deregister(&mut self, registry: &Registry) -> Result<(), Error> { + self.inner.deregister(registry) + } +} + +// This `impl` is useful to send/receive `pidfd` over a UNIX domain socket. +#[cfg(any( + target_os = "android", + target_os = "espidf", + target_os = "fuchsia", + target_os = "hermit", + target_os = "illumos", + target_os = "linux", +))] +impl AsRawFd for Process { + fn as_raw_fd(&self) -> RawFd { + self.inner.as_raw_fd() + } +} + +// This `impl` is useful to send/receive `pidfd` over a UNIX domain socket. +#[cfg(any( + target_os = "android", + target_os = "espidf", + target_os = "fuchsia", + target_os = "hermit", + target_os = "illumos", + target_os = "linux", +))] +impl FromRawFd for Process { + unsafe fn from_raw_fd(fd: RawFd) -> Self { + let inner = sys::Process::from_raw_fd(fd); + Self { inner } + } +} + +// This `impl` is useful to send/receive `pidfd` over a UNIX domain socket. +#[cfg(any( + target_os = "android", + target_os = "espidf", + target_os = "fuchsia", + target_os = "hermit", + target_os = "illumos", + target_os = "linux", +))] +impl IntoRawFd for Process { + fn into_raw_fd(self) -> RawFd { + self.inner.into_raw_fd() + } +} diff --git a/src/sys/unix/mod.rs b/src/sys/unix/mod.rs index 6c0c8850e..1b6a13a50 100644 --- a/src/sys/unix/mod.rs +++ b/src/sys/unix/mod.rs @@ -106,6 +106,19 @@ cfg_os_poll! { // NOTE: the `Waker` type is expected in the selector module as the // `poll(2)` implementation needs to do some special stuff. + #[cfg(feature = "process")] + #[cfg_attr(any( + target_os = "android", + target_os = "espidf", + target_os = "fuchsia", + target_os = "hermit", + target_os = "illumos", + target_os = "linux", + ), path = "process/pidfd.rs")] + mod process; + #[cfg(feature = "process")] + pub use self::process::Process; + mod sourcefd; #[cfg(feature = "os-ext")] pub use self::sourcefd::SourceFd; diff --git a/src/sys/unix/process/pidfd.rs b/src/sys/unix/process/pidfd.rs new file mode 100644 index 000000000..488f0e558 --- /dev/null +++ b/src/sys/unix/process/pidfd.rs @@ -0,0 +1,74 @@ +use std::fs::File; +use std::io::Error; +use std::os::fd::{AsRawFd, FromRawFd, IntoRawFd, RawFd}; +use std::process::Child; + +use libc::{pid_t, SYS_pidfd_open, PIDFD_NONBLOCK}; + +use crate::event::Source; +use crate::{Interest, Registry, Token}; + +#[derive(Debug)] +pub struct Process { + fd: File, +} + +impl Process { + pub fn new(child: &Child) -> Result { + Self::from_pid(child.id() as pid_t) + } + + pub fn from_pid(pid: pid_t) -> Result { + let fd = syscall!(syscall(SYS_pidfd_open, pid, PIDFD_NONBLOCK))?; + // SAFETY: `pidfd_open(2)` ensures the fd is valid. + let fd = unsafe { File::from_raw_fd(fd as RawFd) }; + Ok(Self { fd }) + } +} + +impl AsRawFd for Process { + fn as_raw_fd(&self) -> RawFd { + self.fd.as_raw_fd() + } +} + +impl FromRawFd for Process { + unsafe fn from_raw_fd(fd: RawFd) -> Self { + let fd = File::from_raw_fd(fd); + Self { fd } + } +} + +impl IntoRawFd for Process { + fn into_raw_fd(self) -> RawFd { + self.fd.into_raw_fd() + } +} + +impl Source for Process { + fn register( + &mut self, + registry: &Registry, + token: Token, + interests: Interest, + ) -> Result<(), Error> { + registry + .selector() + .register(self.as_raw_fd(), token, interests) + } + + fn reregister( + &mut self, + registry: &Registry, + token: Token, + interests: Interest, + ) -> Result<(), Error> { + registry + .selector() + .reregister(self.as_raw_fd(), token, interests) + } + + fn deregister(&mut self, registry: &Registry) -> Result<(), Error> { + registry.selector().deregister(self.as_raw_fd()) + } +} diff --git a/tests/process.rs b/tests/process.rs new file mode 100644 index 000000000..9dab9285b --- /dev/null +++ b/tests/process.rs @@ -0,0 +1,77 @@ +#![cfg(all(feature = "os-poll", feature = "net", feature = "process"))] + +use std::process::{Command, Stdio}; + +use mio::{Interest, Process, Token}; + +mod util; + +use util::{expect_events, init_with_poll_with_capacity, ExpectEvent}; + +// Test basic process polling functionality by spawning two child processes. +#[test] +fn child_process() { + let (mut poll, mut events) = init_with_poll_with_capacity(2); + let mut child1 = new_command().spawn().unwrap(); + let mut child2 = new_command().spawn().unwrap(); + let mut p1 = Process::new(&child1).unwrap(); + let mut p2 = Process::new(&child2).unwrap(); + + poll.registry() + .register(&mut p1, ID1, Interest::READABLE) + .unwrap(); + poll.registry() + .register(&mut p2, ID2, Interest::READABLE) + .unwrap(); + + expect_events( + &mut poll, + &mut events, + vec![ + ExpectEvent::new(ID1, Interest::READABLE), + ExpectEvent::new(ID2, Interest::READABLE), + ], + ); + + child1.wait().unwrap(); + child2.wait().unwrap(); +} + +// Test for potential race conditions in process polling by spawning many child processes at once. +#[test] +fn stress_test() { + let num_processes = 1000; + let (mut poll, mut events) = init_with_poll_with_capacity(num_processes); + let mut children = Vec::with_capacity(num_processes); + let mut procs = Vec::with_capacity(num_processes); + let mut expected_events = Vec::with_capacity(num_processes); + + for i in 1..=num_processes { + let child = new_command().spawn().unwrap(); + let mut proc = Process::new(&child).unwrap(); + let token = Token(i); + poll.registry() + .register(&mut proc, token, Interest::READABLE) + .unwrap(); + children.push(child); + procs.push(proc); + expected_events.push(ExpectEvent::new(token, Interest::READABLE)); + } + + expect_events(&mut poll, &mut events, expected_events); + + for mut child in children.into_iter() { + child.wait().unwrap(); + } +} + +// Neutral command to test process spawning. +fn new_command() -> Command { + let mut cmd = Command::new("cargo"); + cmd.arg("--version"); + cmd.stdout(Stdio::null()); + cmd +} + +const ID1: Token = Token(1); +const ID2: Token = Token(2); diff --git a/tests/util/mod.rs b/tests/util/mod.rs index 858c146cc..a899ad31b 100644 --- a/tests/util/mod.rs +++ b/tests/util/mod.rs @@ -10,7 +10,7 @@ use std::ops::BitOr; use std::os::fd::AsRawFd; use std::path::PathBuf; use std::sync::Once; -use std::time::Duration; +use std::time::{Duration, Instant}; use std::{env, fmt, fs, io}; use log::{error, warn}; @@ -32,10 +32,14 @@ pub fn init() { } pub fn init_with_poll() -> (Poll, Events) { + init_with_poll_with_capacity(16) +} + +pub fn init_with_poll_with_capacity(capacity: usize) -> (Poll, Events) { init(); let poll = Poll::new().expect("unable to create Poll instance"); - let events = Events::with_capacity(16); + let events = Events::with_capacity(capacity); (poll, events) } @@ -135,12 +139,20 @@ impl From for Readiness { } pub fn expect_events(poll: &mut Poll, events: &mut Events, mut expected: Vec) { + const TIMEOUT: Duration = Duration::from_millis(500); + const MAX_ITERATIONS: usize = 1000; + // In a lot of calls we expect more then one event, but it could be that // poll returns the first event only in a single call. To be a bit more // lenient we'll poll a couple of times. - for _ in 0..3 { - poll.poll(events, Some(Duration::from_millis(500))) - .expect("unable to poll"); + for _ in 0..MAX_ITERATIONS { + let t = Instant::now(); + poll.poll(events, Some(TIMEOUT)).expect("unable to poll"); + + if t.elapsed() >= TIMEOUT && events.is_empty() { + // Poll timed out. + break; + } for event in events.iter() { let index = expected.iter().position(|expected| expected.matches(event)); From b2f672d018a072cca6a4fd925085bf5022c33edc Mon Sep 17 00:00:00 2001 From: Ivan Gankevich Date: Fri, 27 Dec 2024 17:48:52 +0100 Subject: [PATCH 02/27] Implement more fd traits. Clarify documentation. Add `after_wait` test. --- src/process.rs | 84 +++++++++++++++++------------------ src/sys/unix/process/pidfd.rs | 21 ++++++++- tests/process.rs | 23 +++++++++- 3 files changed, 83 insertions(+), 45 deletions(-) diff --git a/src/process.rs b/src/process.rs index 1d0c76f54..4b0b2834b 100644 --- a/src/process.rs +++ b/src/process.rs @@ -1,13 +1,4 @@ use std::io::Error; -#[cfg(any( - target_os = "android", - target_os = "espidf", - target_os = "fuchsia", - target_os = "hermit", - target_os = "illumos", - target_os = "linux", -))] -use std::os::fd::{AsRawFd, FromRawFd, IntoRawFd, RawFd}; use std::process::Child; use crate::event::Source; @@ -15,16 +6,16 @@ use crate::{sys, Interest, Registry, Token}; /// Process allows polling OS processes for completion. /// -/// When the process exits the event with [`readable`] readiness is generated. +/// When the process exits the event with [`readable`](crate::event::Event::readable) readiness is generated. /// /// # Notes /// /// Events are delivered even if the process has exited by the time [`poll`](crate::Poll::poll) is -/// called. +/// called and has been waited for. /// /// # Implementation notes /// -/// On Linux `Process` uses `pidfd`. +/// On Linux [`Process`] uses `pidfd`. #[derive(Debug)] pub struct Process { inner: sys::Process, @@ -69,7 +60,7 @@ impl Source for Process { } } -// This `impl` is useful to send/receive `pidfd` over a UNIX domain socket. +// The following trait implementations are useful to send/receive `pidfd` over a UNIX-domain socket. #[cfg(any( target_os = "android", target_os = "espidf", @@ -78,39 +69,46 @@ impl Source for Process { target_os = "illumos", target_os = "linux", ))] -impl AsRawFd for Process { - fn as_raw_fd(&self) -> RawFd { - self.inner.as_raw_fd() +mod linux { + use std::os::fd::{AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd, OwnedFd, RawFd}; + + use super::*; + + impl AsFd for Process { + fn as_fd(&self) -> BorrowedFd<'_> { + self.inner.as_fd() + } } -} -// This `impl` is useful to send/receive `pidfd` over a UNIX domain socket. -#[cfg(any( - target_os = "android", - target_os = "espidf", - target_os = "fuchsia", - target_os = "hermit", - target_os = "illumos", - target_os = "linux", -))] -impl FromRawFd for Process { - unsafe fn from_raw_fd(fd: RawFd) -> Self { - let inner = sys::Process::from_raw_fd(fd); - Self { inner } + impl AsRawFd for Process { + fn as_raw_fd(&self) -> RawFd { + self.inner.as_raw_fd() + } } -} -// This `impl` is useful to send/receive `pidfd` over a UNIX domain socket. -#[cfg(any( - target_os = "android", - target_os = "espidf", - target_os = "fuchsia", - target_os = "hermit", - target_os = "illumos", - target_os = "linux", -))] -impl IntoRawFd for Process { - fn into_raw_fd(self) -> RawFd { - self.inner.into_raw_fd() + impl FromRawFd for Process { + unsafe fn from_raw_fd(fd: RawFd) -> Self { + let inner = sys::Process::from_raw_fd(fd); + Self { inner } + } + } + + impl IntoRawFd for Process { + fn into_raw_fd(self) -> RawFd { + self.inner.into_raw_fd() + } + } + + impl From for Process { + fn from(other: OwnedFd) -> Self { + let inner = other.into(); + Self { inner } + } + } + + impl From for OwnedFd { + fn from(other: Process) -> Self { + other.inner.into() + } } } diff --git a/src/sys/unix/process/pidfd.rs b/src/sys/unix/process/pidfd.rs index 488f0e558..0fdb960b0 100644 --- a/src/sys/unix/process/pidfd.rs +++ b/src/sys/unix/process/pidfd.rs @@ -1,6 +1,6 @@ use std::fs::File; use std::io::Error; -use std::os::fd::{AsRawFd, FromRawFd, IntoRawFd, RawFd}; +use std::os::fd::{AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd, OwnedFd, RawFd}; use std::process::Child; use libc::{pid_t, SYS_pidfd_open, PIDFD_NONBLOCK}; @@ -26,6 +26,12 @@ impl Process { } } +impl AsFd for Process { + fn as_fd(&self) -> BorrowedFd<'_> { + self.fd.as_fd() + } +} + impl AsRawFd for Process { fn as_raw_fd(&self) -> RawFd { self.fd.as_raw_fd() @@ -45,6 +51,19 @@ impl IntoRawFd for Process { } } +impl From for Process { + fn from(other: OwnedFd) -> Self { + let fd = other.into(); + Self { fd } + } +} + +impl From for OwnedFd { + fn from(other: Process) -> Self { + other.fd.into() + } +} + impl Source for Process { fn register( &mut self, diff --git a/tests/process.rs b/tests/process.rs index 9dab9285b..f44503274 100644 --- a/tests/process.rs +++ b/tests/process.rs @@ -10,7 +10,7 @@ use util::{expect_events, init_with_poll_with_capacity, ExpectEvent}; // Test basic process polling functionality by spawning two child processes. #[test] -fn child_process() { +fn before_wait() { let (mut poll, mut events) = init_with_poll_with_capacity(2); let mut child1 = new_command().spawn().unwrap(); let mut child2 = new_command().spawn().unwrap(); @@ -37,6 +37,25 @@ fn child_process() { child2.wait().unwrap(); } +// Test that the process that has been waited for still generates read event. +#[test] +fn after_wait() { + let (mut poll, mut events) = init_with_poll_with_capacity(2); + let mut child1 = new_command().spawn().unwrap(); + let mut p1 = Process::new(&child1).unwrap(); + + poll.registry() + .register(&mut p1, ID1, Interest::READABLE) + .unwrap(); + + child1.wait().unwrap(); + expect_events( + &mut poll, + &mut events, + vec![ExpectEvent::new(ID1, Interest::READABLE)], + ); +} + // Test for potential race conditions in process polling by spawning many child processes at once. #[test] fn stress_test() { @@ -69,7 +88,9 @@ fn stress_test() { fn new_command() -> Command { let mut cmd = Command::new("cargo"); cmd.arg("--version"); + cmd.stdin(Stdio::null()); cmd.stdout(Stdio::null()); + cmd.stderr(Stdio::null()); cmd } From d483ab58df8d49142d6042d89be61834e3f77c1c Mon Sep 17 00:00:00 2001 From: Ivan Gankevich Date: Fri, 27 Dec 2024 19:08:29 +0100 Subject: [PATCH 03/27] Implement `Process` fro FreeBSD/MacOS. --- src/process.rs | 2 +- src/sys/unix/mod.rs | 8 ++ src/sys/unix/process/pid.rs | 48 ++++++++++ src/sys/unix/selector/kqueue.rs | 154 +++++++++++++++++++++++--------- tests/process.rs | 6 +- 5 files changed, 169 insertions(+), 49 deletions(-) create mode 100644 src/sys/unix/process/pid.rs diff --git a/src/process.rs b/src/process.rs index 4b0b2834b..634e9406f 100644 --- a/src/process.rs +++ b/src/process.rs @@ -15,7 +15,7 @@ use crate::{sys, Interest, Registry, Token}; /// /// # Implementation notes /// -/// On Linux [`Process`] uses `pidfd`. +/// [`Process`] uses `pidfd` on Linux, `EVFILT_PROC` on MacOS/BSD. #[derive(Debug)] pub struct Process { inner: sys::Process, diff --git a/src/sys/unix/mod.rs b/src/sys/unix/mod.rs index 1b6a13a50..387e3361a 100644 --- a/src/sys/unix/mod.rs +++ b/src/sys/unix/mod.rs @@ -115,6 +115,14 @@ cfg_os_poll! { target_os = "illumos", target_os = "linux", ), path = "process/pidfd.rs")] + #[cfg_attr(any( + target_os = "freebsd", + target_os = "ios", + target_os = "macos", + target_os = "tvos", + target_os = "visionos", + target_os = "watchos", + ), path = "process/pid.rs")] mod process; #[cfg(feature = "process")] pub use self::process::Process; diff --git a/src/sys/unix/process/pid.rs b/src/sys/unix/process/pid.rs new file mode 100644 index 000000000..a35ff8db6 --- /dev/null +++ b/src/sys/unix/process/pid.rs @@ -0,0 +1,48 @@ +use std::io::Error; +use std::process::Child; + +use libc::pid_t; + +use crate::event::Source; +use crate::{Interest, Registry, Token}; + +#[derive(Debug)] +pub struct Process { + pid: pid_t, +} + +impl Process { + pub fn new(child: &Child) -> Result { + Self::from_pid(child.id() as pid_t) + } + + pub fn from_pid(pid: pid_t) -> Result { + Ok(Self { pid }) + } +} + +impl Source for Process { + fn register( + &mut self, + registry: &Registry, + token: Token, + interests: Interest, + ) -> Result<(), Error> { + registry.selector().register_pid(self.pid, token, interests) + } + + fn reregister( + &mut self, + registry: &Registry, + token: Token, + interests: Interest, + ) -> Result<(), Error> { + registry + .selector() + .reregister_pid(self.pid, token, interests) + } + + fn deregister(&mut self, registry: &Registry) -> Result<(), Error> { + registry.selector().deregister_pid(self.pid) + } +} diff --git a/src/sys/unix/selector/kqueue.rs b/src/sys/unix/selector/kqueue.rs index f31db35fb..4c1b84bd3 100644 --- a/src/sys/unix/selector/kqueue.rs +++ b/src/sys/unix/selector/kqueue.rs @@ -124,20 +124,14 @@ impl Selector { pub fn register(&self, fd: RawFd, token: Token, interests: Interest) -> io::Result<()> { let flags = libc::EV_CLEAR | libc::EV_RECEIPT | libc::EV_ADD; // At most we need two changes, but maybe we only need 1. - let mut changes: [MaybeUninit; 2] = - [MaybeUninit::uninit(), MaybeUninit::uninit()]; - let mut n_changes = 0; + let mut changes: Changes<2> = Changes::new(); if interests.is_writable() { - let kevent = kevent!(fd, libc::EVFILT_WRITE, flags, token.0); - changes[n_changes] = MaybeUninit::new(kevent); - n_changes += 1; + changes.push(kevent!(fd, libc::EVFILT_WRITE, flags, token.0)); } if interests.is_readable() { - let kevent = kevent!(fd, libc::EVFILT_READ, flags, token.0); - changes[n_changes] = MaybeUninit::new(kevent); - n_changes += 1; + changes.push(kevent!(fd, libc::EVFILT_READ, flags, token.0)); } // Older versions of macOS (OS X 10.11 and 10.10 have been witnessed) @@ -152,26 +146,16 @@ impl Selector { // instead of propagating it. // // More info can be found at tokio-rs/mio#582. - let changes = unsafe { - // This is safe because we ensure that at least `n_changes` are in - // the array. - slice::from_raw_parts_mut(changes[0].as_mut_ptr(), n_changes) - }; - kevent_register(self.kq.as_raw_fd(), changes, &[libc::EPIPE as i64]) + kevent_register( + self.kq.as_raw_fd(), + changes.as_mut_slice(), + &[libc::EPIPE as i64], + ) } pub fn reregister(&self, fd: RawFd, token: Token, interests: Interest) -> io::Result<()> { - let flags = libc::EV_CLEAR | libc::EV_RECEIPT; - let write_flags = if interests.is_writable() { - flags | libc::EV_ADD - } else { - flags | libc::EV_DELETE - }; - let read_flags = if interests.is_readable() { - flags | libc::EV_ADD - } else { - flags | libc::EV_DELETE - }; + let write_flags = get_reregister_flags(interests.is_writable()); + let read_flags = get_reregister_flags(interests.is_readable()); let mut changes: [libc::kevent; 2] = [ kevent!(fd, libc::EVFILT_WRITE, write_flags, token.0), @@ -265,6 +249,53 @@ impl Selector { } } +#[cfg(feature = "process")] +impl Selector { + pub fn register_pid( + &self, + pid: libc::pid_t, + token: Token, + interests: Interest, + ) -> io::Result<()> { + let flags = libc::EV_CLEAR | libc::EV_RECEIPT | libc::EV_ADD; + let mut changes: Changes<1> = Changes::new(); + + if interests.is_readable() { + let mut kevent = kevent!(pid, libc::EVFILT_PROC, flags, token.0); + // We are only interested in "process exit" event to be consistent with `pidfd`. + kevent.fflags = libc::NOTE_EXIT; + changes.push(kevent); + } + + kevent_register(self.kq.as_raw_fd(), changes.as_mut_slice(), &[]) + } + + pub fn reregister_pid( + &self, + pid: libc::pid_t, + token: Token, + interests: Interest, + ) -> io::Result<()> { + let mut changes: [libc::kevent; 1] = [kevent!( + pid, + libc::EVFILT_PROC, + get_reregister_flags(interests.is_readable()), + token.0 + )]; + kevent_register(self.kq.as_raw_fd(), &mut changes, &[libc::ENOENT as i64]) + } + + pub fn deregister_pid(&self, pid: libc::pid_t) -> io::Result<()> { + let mut changes: [libc::kevent; 1] = [kevent!( + pid, + libc::EVFILT_PROC, + libc::EV_DELETE | libc::EV_RECEIPT, + 0 + )]; + kevent_register(self.kq.as_raw_fd(), &mut changes, &[libc::ENOENT as i64]) + } +} + /// Register `changes` with `kq`ueue. fn kevent_register( kq: RawFd, @@ -308,6 +339,47 @@ fn check_errors(events: &[libc::kevent], ignored_errors: &[i64]) -> io::Result<( Ok(()) } +/// Get `reregister` flags for the specified `interest`. +/// +/// Returns standard flags plus `EV_ADD` when `interest` is true, `EV_DELETE` otherwise. +fn get_reregister_flags(interest: bool) -> u16 { + let flags = libc::EV_CLEAR | libc::EV_RECEIPT; + if interest { + flags | libc::EV_ADD + } else { + flags | libc::EV_DELETE + }; + flags +} + +struct Changes { + changes: [MaybeUninit; N], + n_changes: usize, +} + +impl Changes { + fn new() -> Self { + Self { + changes: [MaybeUninit::uninit(); N], + n_changes: 0, + } + } + + fn push(&mut self, kevent: libc::kevent) { + debug_assert!(self.n_changes < N, "too many events, increase `N`"); + self.changes[self.n_changes] = MaybeUninit::new(kevent); + self.n_changes += 1; + } + + fn as_mut_slice(&mut self) -> &mut [libc::kevent] { + unsafe { + // This is safe because we ensure that at least `n_changes` are in + // the array. + slice::from_raw_parts_mut(self.changes[0].as_mut_ptr(), self.n_changes) + } + } +} + cfg_io_source! { #[cfg(debug_assertions)] impl Selector { @@ -367,32 +439,26 @@ pub mod event { } pub fn is_readable(event: &Event) -> bool { - event.filter == libc::EVFILT_READ || { - #[cfg(any( - target_os = "freebsd", - target_os = "ios", - target_os = "macos", - target_os = "tvos", - target_os = "visionos", - target_os = "watchos" - ))] + match event.filter { + // Emit process's read event *only* on process exit + // to make `EVFILT_PROC` behaviour consistent with `pidfd`. + #[cfg(feature = "process")] + libc::EVFILT_PROC if (event.fflags & libc::NOTE_EXIT) != 0 => true, + // File descriptor's read event. + libc::EVFILT_READ => true, // Used by the `Awakener`. On platforms that use `eventfd` or a unix // pipe it will emit a readable event so we'll fake that here as // well. - { - event.filter == libc::EVFILT_USER - } - #[cfg(not(any( + #[cfg(any( target_os = "freebsd", target_os = "ios", target_os = "macos", target_os = "tvos", target_os = "visionos", target_os = "watchos" - )))] - { - false - } + ))] + libc::EVFILT_USER => true, + _ => false, } } @@ -404,7 +470,7 @@ pub mod event { (event.flags & libc::EV_ERROR) != 0 || // When the read end of the socket is closed, EV_EOF is set on // flags, and fflags contains the error if there is one. - (event.flags & libc::EV_EOF) != 0 && event.fflags != 0 + (event.flags & libc::EV_EOF) != 0 && event.fflags != 0 && event.filter != libc::EVFILT_PROC } pub fn is_read_closed(event: &Event) -> bool { diff --git a/tests/process.rs b/tests/process.rs index f44503274..0a59d87b8 100644 --- a/tests/process.rs +++ b/tests/process.rs @@ -1,11 +1,9 @@ #![cfg(all(feature = "os-poll", feature = "net", feature = "process"))] -use std::process::{Command, Stdio}; - use mio::{Interest, Process, Token}; +use std::process::{Command, Stdio}; mod util; - use util::{expect_events, init_with_poll_with_capacity, ExpectEvent}; // Test basic process polling functionality by spawning two child processes. @@ -59,7 +57,7 @@ fn after_wait() { // Test for potential race conditions in process polling by spawning many child processes at once. #[test] fn stress_test() { - let num_processes = 1000; + let num_processes = 100; let (mut poll, mut events) = init_with_poll_with_capacity(num_processes); let mut children = Vec::with_capacity(num_processes); let mut procs = Vec::with_capacity(num_processes); From e73f5c114320ef86dab93fdb170b39a43f379f53 Mon Sep 17 00:00:00 2001 From: Ivan Gankevich Date: Fri, 27 Dec 2024 19:18:34 +0100 Subject: [PATCH 04/27] Add dummy `Process` implementation. --- src/macros.rs | 11 +++++ src/sys/shell/mod.rs | 5 +++ src/sys/shell/process.rs | 83 +++++++++++++++++++++++++++++++++++ src/sys/unix/process/pid.rs | 8 ++-- src/sys/unix/process/pidfd.rs | 8 ++-- 5 files changed, 105 insertions(+), 10 deletions(-) create mode 100644 src/sys/shell/process.rs diff --git a/src/macros.rs b/src/macros.rs index e380c6b14..e6ea9863f 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -69,6 +69,17 @@ macro_rules! cfg_any_os_ext { } } +/// The `process` feature is enabled. +macro_rules! cfg_process { + ($($item:item)*) => { + $( + #[cfg(feature = "process")] + #[cfg_attr(docsrs, doc(cfg(feature = "process")))] + $item + )* + } +} + macro_rules! trace { ($($t:tt)*) => { log!(trace, $($t)*) diff --git a/src/sys/shell/mod.rs b/src/sys/shell/mod.rs index aa1c6220f..151225ea5 100644 --- a/src/sys/shell/mod.rs +++ b/src/sys/shell/mod.rs @@ -7,6 +7,11 @@ macro_rules! os_required { mod selector; pub(crate) use self::selector::{event, Event, Events, Selector}; +cfg_process! { + mod process; + pub(crate) use self::process::Process; +} + #[cfg(not(target_os = "wasi"))] mod waker; #[cfg(not(target_os = "wasi"))] diff --git a/src/sys/shell/process.rs b/src/sys/shell/process.rs new file mode 100644 index 000000000..b84d36d86 --- /dev/null +++ b/src/sys/shell/process.rs @@ -0,0 +1,83 @@ +use crate::event::Source; +use crate::{Interest, Registry, Token}; +use libc::pid_t; +use std::io::Error; +use std::process::Child; + +#[derive(Debug)] +pub struct Process {} + +impl Process { + pub fn new(_: &Child) -> Result { + os_required!() + } + + #[cfg(unix)] + pub fn from_pid(_: pid_t) -> Result { + os_required!() + } +} + +#[cfg(any( + target_os = "android", + target_os = "espidf", + target_os = "fuchsia", + target_os = "hermit", + target_os = "illumos", + target_os = "linux", +))] +mod linux { + use std::os::fd::{AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd, OwnedFd, RawFd}; + + use super::*; + + impl AsFd for Process { + fn as_fd(&self) -> BorrowedFd<'_> { + os_required!() + } + } + + impl AsRawFd for Process { + fn as_raw_fd(&self) -> RawFd { + os_required!() + } + } + + impl FromRawFd for Process { + unsafe fn from_raw_fd(_: RawFd) -> Self { + os_required!() + } + } + + impl IntoRawFd for Process { + fn into_raw_fd(self) -> RawFd { + os_required!() + } + } + + impl From for Process { + fn from(_: OwnedFd) -> Self { + os_required!() + } + } + + impl From for OwnedFd { + fn from(_: Process) -> Self { + os_required!() + } + } +} + +impl Source for Process { + fn register(&mut self, _: &Registry, _: Token, _: Interest) -> Result<(), Error> { + os_required!() + } + + fn reregister(&mut self, _: &Registry, _: Token, _: Interest) -> Result<(), Error> { + os_required!() + } + + fn deregister(&mut self, _: &Registry) -> Result<(), Error> { + os_required!() + } +} diff --git a/src/sys/unix/process/pid.rs b/src/sys/unix/process/pid.rs index a35ff8db6..913a54680 100644 --- a/src/sys/unix/process/pid.rs +++ b/src/sys/unix/process/pid.rs @@ -1,10 +1,8 @@ -use std::io::Error; -use std::process::Child; - -use libc::pid_t; - use crate::event::Source; use crate::{Interest, Registry, Token}; +use libc::pid_t; +use std::io::Error; +use std::process::Child; #[derive(Debug)] pub struct Process { diff --git a/src/sys/unix/process/pidfd.rs b/src/sys/unix/process/pidfd.rs index 0fdb960b0..c895c0f25 100644 --- a/src/sys/unix/process/pidfd.rs +++ b/src/sys/unix/process/pidfd.rs @@ -1,13 +1,11 @@ +use crate::event::Source; +use crate::{Interest, Registry, Token}; +use libc::{pid_t, SYS_pidfd_open, PIDFD_NONBLOCK}; use std::fs::File; use std::io::Error; use std::os::fd::{AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd, OwnedFd, RawFd}; use std::process::Child; -use libc::{pid_t, SYS_pidfd_open, PIDFD_NONBLOCK}; - -use crate::event::Source; -use crate::{Interest, Registry, Token}; - #[derive(Debug)] pub struct Process { fd: File, From b3f95a9d1b330d1e73b36acd36a577ae993a39b0 Mon Sep 17 00:00:00 2001 From: Ivan Gankevich Date: Fri, 27 Dec 2024 19:26:14 +0100 Subject: [PATCH 05/27] Hermit workarounds. --- src/process.rs | 5 +++++ src/sys/unix/process/pidfd.rs | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/src/process.rs b/src/process.rs index 634e9406f..413dedb49 100644 --- a/src/process.rs +++ b/src/process.rs @@ -70,7 +70,12 @@ impl Source for Process { target_os = "linux", ))] mod linux { + #[cfg(not(target_os = "hermit"))] use std::os::fd::{AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd, OwnedFd, RawFd}; + // TODO: once is fixed this + // can use `std::os::fd` and be merged with the above. + #[cfg(target_os = "hermit")] + use std::os::hermit::io::{AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd, OwnedFd, RawFd}; use super::*; diff --git a/src/sys/unix/process/pidfd.rs b/src/sys/unix/process/pidfd.rs index c895c0f25..7907470d0 100644 --- a/src/sys/unix/process/pidfd.rs +++ b/src/sys/unix/process/pidfd.rs @@ -3,7 +3,12 @@ use crate::{Interest, Registry, Token}; use libc::{pid_t, SYS_pidfd_open, PIDFD_NONBLOCK}; use std::fs::File; use std::io::Error; +#[cfg(not(target_os = "hermit"))] use std::os::fd::{AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd, OwnedFd, RawFd}; +// TODO: once is fixed this +// can use `std::os::fd` and be merged with the above. +#[cfg(target_os = "hermit")] +use std::os::hermit::io::{AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd, OwnedFd, RawFd}; use std::process::Child; #[derive(Debug)] From 146a594ee2f0daef779cba48d266f60a317d0a6e Mon Sep 17 00:00:00 2001 From: Ivan Gankevich Date: Fri, 27 Dec 2024 19:31:16 +0100 Subject: [PATCH 06/27] Rename `process` feature to `os-proc`. --- Cargo.toml | 4 ++-- src/lib.rs | 9 +++++---- src/macros.rs | 8 ++++---- src/sys/shell/mod.rs | 2 +- src/sys/unix/mod.rs | 4 ++-- src/sys/unix/selector/kqueue.rs | 4 ++-- tests/process.rs | 2 +- 7 files changed, 17 insertions(+), 16 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 096ace739..fa226fc3e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -40,10 +40,10 @@ os-ext = [ "windows-sys/Win32_System_Pipes", "windows-sys/Win32_Security", ] +# Enables OS process polling. +os-proc = ["os-poll"] # Enables `mio::net` module containing networking primitives. net = [] -# Enables OS process polling. -process = [] [dependencies] log = { version = "0.4.8", optional = true } diff --git a/src/lib.rs b/src/lib.rs index 96400a7da..cb53ff903 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -46,8 +46,6 @@ mod macros; mod interest; mod poll; -#[cfg(feature = "process")] -mod process; mod sys; mod token; #[cfg(not(target_os = "wasi"))] @@ -63,6 +61,11 @@ cfg_net! { pub mod net; } +cfg_os_proc! { + mod process; + pub use process::Process; +} + #[doc(no_inline)] pub use event::Events; pub use interest::Interest; @@ -70,8 +73,6 @@ pub use poll::{Poll, Registry}; pub use token::Token; #[cfg(not(target_os = "wasi"))] pub use waker::Waker; -#[cfg(feature = "process")] -pub use process::Process; #[cfg(all(unix, feature = "os-ext"))] #[cfg_attr(docsrs, doc(cfg(all(unix, feature = "os-ext"))))] diff --git a/src/macros.rs b/src/macros.rs index e6ea9863f..0f147a674 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -69,12 +69,12 @@ macro_rules! cfg_any_os_ext { } } -/// The `process` feature is enabled. -macro_rules! cfg_process { +/// The `os-proc` feature is enabled. +macro_rules! cfg_os_proc { ($($item:item)*) => { $( - #[cfg(feature = "process")] - #[cfg_attr(docsrs, doc(cfg(feature = "process")))] + #[cfg(feature = "os-proc")] + #[cfg_attr(docsrs, doc(cfg(feature = "os-proc")))] $item )* } diff --git a/src/sys/shell/mod.rs b/src/sys/shell/mod.rs index 151225ea5..fc08ed023 100644 --- a/src/sys/shell/mod.rs +++ b/src/sys/shell/mod.rs @@ -7,7 +7,7 @@ macro_rules! os_required { mod selector; pub(crate) use self::selector::{event, Event, Events, Selector}; -cfg_process! { +cfg_os_proc! { mod process; pub(crate) use self::process::Process; } diff --git a/src/sys/unix/mod.rs b/src/sys/unix/mod.rs index 387e3361a..6067bf484 100644 --- a/src/sys/unix/mod.rs +++ b/src/sys/unix/mod.rs @@ -106,7 +106,7 @@ cfg_os_poll! { // NOTE: the `Waker` type is expected in the selector module as the // `poll(2)` implementation needs to do some special stuff. - #[cfg(feature = "process")] + #[cfg(feature = "os-proc")] #[cfg_attr(any( target_os = "android", target_os = "espidf", @@ -124,7 +124,7 @@ cfg_os_poll! { target_os = "watchos", ), path = "process/pid.rs")] mod process; - #[cfg(feature = "process")] + #[cfg(feature = "os-proc")] pub use self::process::Process; mod sourcefd; diff --git a/src/sys/unix/selector/kqueue.rs b/src/sys/unix/selector/kqueue.rs index 4c1b84bd3..b0b71dc44 100644 --- a/src/sys/unix/selector/kqueue.rs +++ b/src/sys/unix/selector/kqueue.rs @@ -249,7 +249,7 @@ impl Selector { } } -#[cfg(feature = "process")] +#[cfg(feature = "os-proc")] impl Selector { pub fn register_pid( &self, @@ -442,7 +442,7 @@ pub mod event { match event.filter { // Emit process's read event *only* on process exit // to make `EVFILT_PROC` behaviour consistent with `pidfd`. - #[cfg(feature = "process")] + #[cfg(feature = "os-proc")] libc::EVFILT_PROC if (event.fflags & libc::NOTE_EXIT) != 0 => true, // File descriptor's read event. libc::EVFILT_READ => true, diff --git a/tests/process.rs b/tests/process.rs index 0a59d87b8..b938d8da6 100644 --- a/tests/process.rs +++ b/tests/process.rs @@ -1,4 +1,4 @@ -#![cfg(all(feature = "os-poll", feature = "net", feature = "process"))] +#![cfg(all(feature = "os-poll", feature = "net", feature = "os-proc"))] use mio::{Interest, Process, Token}; use std::process::{Command, Stdio}; From 5e2c3cdfdcc7f5be85023454c362fa0194a88c89 Mon Sep 17 00:00:00 2001 From: Ivan Gankevich Date: Fri, 27 Dec 2024 21:23:32 +0100 Subject: [PATCH 07/27] Implement `Process` for windows. --- Cargo.toml | 7 +- src/lib.rs | 5 + src/process.rs | 69 +++++++++-- src/sys/unix/mod.rs | 40 +++---- src/sys/windows/afd.rs | 2 +- src/sys/windows/mod.rs | 5 + src/sys/windows/named_pipe.rs | 2 +- src/sys/windows/process.rs | 218 ++++++++++++++++++++++++++++++++++ src/sys/windows/selector.rs | 33 ++++- 9 files changed, 347 insertions(+), 34 deletions(-) create mode 100644 src/sys/windows/process.rs diff --git a/Cargo.toml b/Cargo.toml index fa226fc3e..d3a98cd8e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -41,7 +41,12 @@ os-ext = [ "windows-sys/Win32_Security", ] # Enables OS process polling. -os-proc = ["os-poll"] +os-proc = [ + "os-poll", + "windows-sys/Win32_Security", + "windows-sys/Win32_System_JobObjects", + "windows-sys/Win32_System_SystemServices", +] # Enables `mio::net` module containing networking primitives. net = [] diff --git a/src/lib.rs b/src/lib.rs index cb53ff903..0cd96c28b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -126,6 +126,11 @@ pub mod features { //! `os-ext` enables additional OS specific facilities. These facilities can //! be found in the `unix` and `windows` module. //! + #![cfg_attr(feature = "os-proc", doc = "## `os-proc` (enabled)")] + #![cfg_attr(not(feature = "os-proc"), doc = "## `os-proc` (disabled)")] + //! + //! `os-proc` enables OS process polling via `Process`. + //! #![cfg_attr(feature = "net", doc = "## Network types (enabled)")] #![cfg_attr(not(feature = "net"), doc = "## Network types (disabled)")] //! diff --git a/src/process.rs b/src/process.rs index 413dedb49..cc0708c09 100644 --- a/src/process.rs +++ b/src/process.rs @@ -1,8 +1,9 @@ -use std::io::Error; -use std::process::Child; - use crate::event::Source; use crate::{sys, Interest, Registry, Token}; +use std::io::Error; +#[cfg(windows)] +use std::os::windows::io::{AsRawHandle, RawHandle}; +use std::process::Child; /// Process allows polling OS processes for completion. /// @@ -15,7 +16,8 @@ use crate::{sys, Interest, Registry, Token}; /// /// # Implementation notes /// -/// [`Process`] uses `pidfd` on Linux, `EVFILT_PROC` on MacOS/BSD. +/// [`Process`] uses `pidfd` on Linux, `EVFILT_PROC` on MacOS/BSD and `AssignProcessToJobObject` on +/// Windows. #[derive(Debug)] pub struct Process { inner: sys::Process, @@ -24,7 +26,7 @@ pub struct Process { impl Process { /// Create new process from [`Child`](std::process::Child). pub fn new(child: &Child) -> Result { - let inner = sys::Process::new(child)?; + let inner = sys::Process::new(child.as_raw_handle())?; Ok(Self { inner }) } @@ -34,6 +36,13 @@ impl Process { let inner = sys::Process::from_pid(pid)?; Ok(Self { inner }) } + + /// Create new process from the process handle. + #[cfg(windows)] + pub fn from_handle(child: RawHandle) -> Result { + let inner = sys::Process::new(child)?; + Ok(Self { inner }) + } } impl Source for Process { @@ -70,6 +79,7 @@ impl Source for Process { target_os = "linux", ))] mod linux { + use super::*; #[cfg(not(target_os = "hermit"))] use std::os::fd::{AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd, OwnedFd, RawFd}; // TODO: once is fixed this @@ -77,8 +87,6 @@ mod linux { #[cfg(target_os = "hermit")] use std::os::hermit::io::{AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd, OwnedFd, RawFd}; - use super::*; - impl AsFd for Process { fn as_fd(&self) -> BorrowedFd<'_> { self.inner.as_fd() @@ -117,3 +125,50 @@ mod linux { } } } + +#[cfg(windows)] +#[cfg_attr(docsrs, doc(cfg(windows)))] +mod windows { + use super::*; + use std::os::windows::io::{ + AsHandle, AsRawHandle, BorrowedHandle, FromRawHandle, IntoRawHandle, OwnedHandle, RawHandle, + }; + + impl AsRawHandle for Process { + fn as_raw_handle(&self) -> RawHandle { + self.inner.as_raw_handle() + } + } + + impl AsHandle for Process { + fn as_handle(&self) -> BorrowedHandle<'_> { + self.inner.as_handle() + } + } + + impl FromRawHandle for Process { + unsafe fn from_raw_handle(job: RawHandle) -> Self { + let inner = sys::Process::from_raw_handle(job); + Self { inner } + } + } + + impl IntoRawHandle for Process { + fn into_raw_handle(self) -> RawHandle { + self.inner.into_raw_handle() + } + } + + impl From for OwnedHandle { + fn from(other: Process) -> Self { + other.inner.into() + } + } + + impl From for Process { + fn from(other: OwnedHandle) -> Self { + let inner = other.into(); + Self { inner } + } + } +} diff --git a/src/sys/unix/mod.rs b/src/sys/unix/mod.rs index 6067bf484..84f3b6c78 100644 --- a/src/sys/unix/mod.rs +++ b/src/sys/unix/mod.rs @@ -106,26 +106,26 @@ cfg_os_poll! { // NOTE: the `Waker` type is expected in the selector module as the // `poll(2)` implementation needs to do some special stuff. - #[cfg(feature = "os-proc")] - #[cfg_attr(any( - target_os = "android", - target_os = "espidf", - target_os = "fuchsia", - target_os = "hermit", - target_os = "illumos", - target_os = "linux", - ), path = "process/pidfd.rs")] - #[cfg_attr(any( - target_os = "freebsd", - target_os = "ios", - target_os = "macos", - target_os = "tvos", - target_os = "visionos", - target_os = "watchos", - ), path = "process/pid.rs")] - mod process; - #[cfg(feature = "os-proc")] - pub use self::process::Process; + cfg_os_proc! { + #[cfg_attr(any( + target_os = "android", + target_os = "espidf", + target_os = "fuchsia", + target_os = "hermit", + target_os = "illumos", + target_os = "linux", + ), path = "process/pidfd.rs")] + #[cfg_attr(any( + target_os = "freebsd", + target_os = "ios", + target_os = "macos", + target_os = "tvos", + target_os = "visionos", + target_os = "watchos", + ), path = "process/pid.rs")] + mod process; + pub use self::process::Process; + } mod sourcefd; #[cfg(feature = "os-ext")] diff --git a/src/sys/windows/afd.rs b/src/sys/windows/afd.rs index 11373cfca..34cf81e02 100644 --- a/src/sys/windows/afd.rs +++ b/src/sys/windows/afd.rs @@ -205,7 +205,7 @@ cfg_io_source! { // Non-AFD types (currently only NamedPipe), use odd numbered // tokens. This allows the selector to differentiate between them // and dispatch events accordingly. - let token = NEXT_TOKEN.fetch_add(2, Ordering::Relaxed) + 2; + let token = NEXT_TOKEN.fetch_add(3, Ordering::Relaxed) + 3; let afd = Afd { fd }; cp.add_handle(token, &afd.fd)?; match SetFileCompletionNotificationModes( diff --git a/src/sys/windows/mod.rs b/src/sys/windows/mod.rs index 89d74b1a2..859953908 100644 --- a/src/sys/windows/mod.rs +++ b/src/sys/windows/mod.rs @@ -46,6 +46,11 @@ cfg_os_ext! { mod waker; pub(crate) use waker::Waker; +cfg_os_proc! { + mod process; + pub(crate) use process::Process; +} + cfg_io_source! { use std::io; use std::os::windows::io::RawSocket; diff --git a/src/sys/windows/named_pipe.rs b/src/sys/windows/named_pipe.rs index eb35d3797..e018d3fde 100644 --- a/src/sys/windows/named_pipe.rs +++ b/src/sys/windows/named_pipe.rs @@ -622,7 +622,7 @@ impl Source for NamedPipe { io.cp = Some(selector.clone_port()); - let inner_token = NEXT_TOKEN.fetch_add(2, Relaxed) + 2; + let inner_token = NEXT_TOKEN.fetch_add(3, Relaxed) + 3; selector.inner.cp.add_handle(inner_token, self)?; } diff --git a/src/sys/windows/process.rs b/src/sys/windows/process.rs new file mode 100644 index 000000000..5dc7e8ba4 --- /dev/null +++ b/src/sys/windows/process.rs @@ -0,0 +1,218 @@ +use crate::event::Source; +use crate::sys::windows::iocp::CompletionPort; +use crate::{Interest, Registry, Token}; +use std::collections::HashMap; +use std::io; +use std::mem::size_of; +use std::os::windows::io::{ + AsHandle, AsRawHandle, BorrowedHandle, FromRawHandle, IntoRawHandle, OwnedHandle, RawHandle, +}; +use std::sync::atomic::AtomicUsize; +use std::sync::atomic::Ordering::Relaxed; +use std::sync::{Arc, LazyLock, Mutex}; +use windows_sys::Win32::System::JobObjects::{ + AssignProcessToJobObject, CreateJobObjectW, JobObjectAssociateCompletionPortInformation, + SetInformationJobObject, JOBOBJECT_ASSOCIATE_COMPLETION_PORT, +}; + +// Odd tokens are for named pipes +static NEXT_TOKEN: AtomicUsize = AtomicUsize::new(2); +// inner token -> token +pub(crate) static TOKEN_MAPPING: LazyLock>> = + LazyLock::new(|| Mutex::new(HashMap::new())); + +// https://devblogs.microsoft.com/oldnewthing/20130405-00/?p=4743 +#[derive(Debug)] +pub struct Process { + job: OwnedHandle, + io: Mutex, +} + +impl Process { + pub fn new(child: RawHandle) -> io::Result { + let job = new_job()?; + assign_process_to_job(job.as_raw_handle(), child)?; + let io = Mutex::new(Io { + cp: None, + token: None, + }); + Ok(Self { job, io }) + } +} + +impl AsRawHandle for Process { + fn as_raw_handle(&self) -> RawHandle { + self.job.as_raw_handle() + } +} + +impl AsHandle for Process { + fn as_handle(&self) -> BorrowedHandle<'_> { + self.job.as_handle() + } +} + +impl FromRawHandle for Process { + unsafe fn from_raw_handle(job: RawHandle) -> Self { + let io = Mutex::new(Io { + cp: None, + token: None, + }); + let job = OwnedHandle::from_raw_handle(job); + Self { job, io } + } +} + +impl IntoRawHandle for Process { + fn into_raw_handle(self) -> RawHandle { + self.job.into_raw_handle() + } +} + +impl From for OwnedHandle { + fn from(other: Process) -> Self { + other.job.into() + } +} + +impl From for Process { + fn from(other: OwnedHandle) -> Self { + let job = other.into(); + let io = Mutex::new(Io { + cp: None, + token: None, + }); + Self { job, io } + } +} + +impl Source for Process { + fn register(&mut self, registry: &Registry, token: Token, _: Interest) -> io::Result<()> { + let mut io = self.io.lock().unwrap(); + + io.check_association(registry, false)?; + + if io.token.is_some() { + return Err(io::Error::new( + io::ErrorKind::AlreadyExists, + "I/O source already registered with a `Registry`", + )); + } + + if io.cp.is_none() { + let selector = registry.selector(); + + let port = selector.clone_port(); + + let inner_token = NEXT_TOKEN.fetch_add(3, Relaxed) + 3; + TOKEN_MAPPING.lock().unwrap().insert(inner_token, token); + associate_job_with_completion_port( + self.job.as_raw_handle(), + port.as_raw_handle(), + inner_token, + )?; + + io.cp = Some(port); + } + + io.token = Some(token); + drop(io); + + Ok(()) + } + + fn reregister(&mut self, registry: &Registry, token: Token, _: Interest) -> io::Result<()> { + let mut io = self.io.lock().unwrap(); + + io.check_association(registry, true)?; + + io.token = Some(token); + drop(io); + + Ok(()) + } + + fn deregister(&mut self, registry: &Registry) -> io::Result<()> { + let mut io = self.io.lock().unwrap(); + + io.check_association(registry, true)?; + + if io.token.is_none() { + return Err(io::Error::new( + io::ErrorKind::NotFound, + "I/O source not registered with `Registry`", + )); + } + + io.token = None; + Ok(()) + } +} + +#[derive(Debug)] +struct Io { + // Uniquely identifies the selector associated with this named pipe + cp: Option>, + // Token used to identify events + token: Option, +} + +impl Io { + fn check_association(&self, registry: &Registry, required: bool) -> io::Result<()> { + match self.cp { + Some(ref cp) if !registry.selector().same_port(cp) => Err(io::Error::new( + io::ErrorKind::AlreadyExists, + "I/O source already registered with a different `Registry`", + )), + None if required => Err(io::Error::new( + io::ErrorKind::NotFound, + "I/O source not registered with `Registry`", + )), + _ => Ok(()), + } + } +} + +fn new_job() -> io::Result { + // SAFETY: `CreateJobObjectW` returns null pointer on failure and a valid job handle otherwise. + let handle = unsafe { CreateJobObjectW(std::ptr::null(), std::ptr::null()) }; + if handle == 0 { + return Err(io::Error::last_os_error()); + } + // SAFETY: `CreateJobObjectW` returns a valid handle on success. + let job = unsafe { OwnedHandle::from_raw_handle(handle as _) }; + Ok(job) +} + +fn assign_process_to_job(job: RawHandle, child: RawHandle) -> io::Result<()> { + if unsafe { AssignProcessToJobObject(job as _, child as _) } == 0 { + Err(io::Error::last_os_error()) + } else { + Ok(()) + } +} + +fn associate_job_with_completion_port( + job: RawHandle, + port: RawHandle, + key: usize, +) -> io::Result<()> { + let job_port = JOBOBJECT_ASSOCIATE_COMPLETION_PORT { + CompletionKey: key as _, + CompletionPort: port as _, + }; + // SAFETY: We provide valid `job` and `port` handles. + if unsafe { + SetInformationJobObject( + job as _, + JobObjectAssociateCompletionPortInformation, + std::ptr::from_ref(&job_port) as _, + size_of::() as u32, + ) + } == 0 + { + Err(io::Error::last_os_error()) + } else { + Ok(()) + } +} diff --git a/src/sys/windows/selector.rs b/src/sys/windows/selector.rs index 7faeb34b6..1e49e421c 100644 --- a/src/sys/windows/selector.rs +++ b/src/sys/windows/selector.rs @@ -1,7 +1,11 @@ use super::afd::{self, Afd, AfdPollInfo}; use super::io_status_block::IoStatusBlock; use super::Event; +use crate::sys::windows::process::TOKEN_MAPPING; use crate::sys::Events; +use windows_sys::Win32::System::SystemServices::{ + JOB_OBJECT_MSG_ABNORMAL_EXIT_PROCESS, JOB_OBJECT_MSG_EXIT_PROCESS, +}; cfg_net! { use crate::sys::event::{ @@ -363,7 +367,7 @@ impl Selector { self.inner.cp.clone() } - #[cfg(feature = "os-ext")] + #[cfg(any(feature = "os-ext", feature = "os-proc"))] pub(super) fn same_port(&self, other: &Arc) -> bool { Arc::ptr_eq(&self.inner.cp, other) } @@ -491,11 +495,29 @@ impl SelectorInner { let mut n = 0; let mut update_queue = self.update_queue.lock().unwrap(); for iocp_event in iocp_events.iter() { - if iocp_event.overlapped().is_null() { + if iocp_event.token() % 3 == 2 { + // Process + match iocp_event.bytes_transferred() { + // We are only interested in "process exit" events to be consistent with `pidfd`. + JOB_OBJECT_MSG_EXIT_PROCESS | JOB_OBJECT_MSG_ABNORMAL_EXIT_PROCESS => { + // lpOverlapped is the process id + eprintln!("feed_events {}", iocp_event.bytes_transferred()); + if let Some(token) = TOKEN_MAPPING.lock().unwrap().get(&iocp_event.token()) + { + let mut event = Event::new(*token); + event.set_readable(); + events.push(event); + n += 1; + } + } + _ => {} + } + continue; + } else if iocp_event.overlapped().is_null() { events.push(Event::from_completion_status(iocp_event)); n += 1; continue; - } else if iocp_event.token() % 2 == 1 { + } else if iocp_event.token() % 3 == 1 { // Handle is a named pipe. This could be extended to be any non-AFD event. let callback = (*(iocp_event.overlapped() as *mut super::Overlapped)).callback; @@ -695,13 +717,16 @@ impl Drop for SelectorInner { for iocp_event in iocp_events.iter() { if iocp_event.overlapped().is_null() { // Custom event - } else if iocp_event.token() % 2 == 1 { + } else if iocp_event.token() % 3 == 1 { // Named pipe, dispatch the event so it can release resources let callback = unsafe { (*(iocp_event.overlapped() as *mut super::Overlapped)).callback }; callback(iocp_event.entry(), None); + } else if iocp_event.token() % 3 == 2 { + // Process + eprintln!("SelectorInner::drop"); } else { // drain sock state to release memory of Arc reference let _sock_state = from_overlapped(iocp_event.overlapped()); From 41251102a18337597f9c5a2bd984843e77f0d6f5 Mon Sep 17 00:00:00 2001 From: Ivan Gankevich Date: Fri, 27 Dec 2024 23:09:28 +0100 Subject: [PATCH 08/27] Refactor `NEXT_TOKEN` into per-registry token mapping. --- src/process.rs | 13 +--- src/sys/windows/afd.rs | 12 +-- src/sys/windows/mod.rs | 1 + src/sys/windows/named_pipe.rs | 14 ++-- src/sys/windows/process.rs | 104 ++++++-------------------- src/sys/windows/selector.rs | 137 ++++++++++++++++++++++++++++------ tests/util/mod.rs | 3 +- 7 files changed, 149 insertions(+), 135 deletions(-) diff --git a/src/process.rs b/src/process.rs index cc0708c09..94617553e 100644 --- a/src/process.rs +++ b/src/process.rs @@ -11,8 +11,10 @@ use std::process::Child; /// /// # Notes /// -/// Events are delivered even if the process has exited by the time [`poll`](crate::Poll::poll) is -/// called and has been waited for. +/// Events are delivered even if the process has exited and has been waited for +/// by the time [`poll`](crate::Poll::poll) is called. +/// +/// On Windows a process might be registered with at most one poller. /// /// # Implementation notes /// @@ -36,13 +38,6 @@ impl Process { let inner = sys::Process::from_pid(pid)?; Ok(Self { inner }) } - - /// Create new process from the process handle. - #[cfg(windows)] - pub fn from_handle(child: RawHandle) -> Result { - let inner = sys::Process::new(child)?; - Ok(Self { inner }) - } } impl Source for Process { diff --git a/src/sys/windows/afd.rs b/src/sys/windows/afd.rs index 34cf81e02..412302081 100644 --- a/src/sys/windows/afd.rs +++ b/src/sys/windows/afd.rs @@ -117,7 +117,6 @@ cfg_io_source! { use std::mem::zeroed; use std::os::windows::io::{FromRawHandle, RawHandle}; use std::ptr::null_mut; - use std::sync::atomic::{AtomicUsize, Ordering}; use windows_sys::Wdk::Foundation::OBJECT_ATTRIBUTES; use windows_sys::Wdk::Storage::FileSystem::{NtCreateFile, FILE_OPEN}; @@ -162,8 +161,6 @@ cfg_io_source! { 'o' as _ ]; - static NEXT_TOKEN: AtomicUsize = AtomicUsize::new(0); - impl AfdPollInfo { pub fn zeroed() -> AfdPollInfo { unsafe { zeroed() } @@ -172,7 +169,7 @@ cfg_io_source! { impl Afd { /// Create new Afd instance. - pub(crate) fn new(cp: &CompletionPort) -> io::Result { + pub(crate) fn new(cp: &CompletionPort, inner_token: usize) -> io::Result { let mut afd_helper_handle: HANDLE = INVALID_HANDLE_VALUE; let mut iosb = IO_STATUS_BLOCK { Anonymous: IO_STATUS_BLOCK_0 { Status: 0 }, @@ -201,13 +198,8 @@ cfg_io_source! { return Err(io::Error::new(raw_err.kind(), msg)); } let fd = File::from_raw_handle(afd_helper_handle as RawHandle); - // Increment by 2 to reserve space for other types of handles. - // Non-AFD types (currently only NamedPipe), use odd numbered - // tokens. This allows the selector to differentiate between them - // and dispatch events accordingly. - let token = NEXT_TOKEN.fetch_add(3, Ordering::Relaxed) + 3; let afd = Afd { fd }; - cp.add_handle(token, &afd.fd)?; + cp.add_handle(inner_token, &afd.fd)?; match SetFileCompletionNotificationModes( afd_helper_handle, FILE_SKIP_SET_EVENT_ON_HANDLE as u8 // This is just 2, so fits in u8 diff --git a/src/sys/windows/mod.rs b/src/sys/windows/mod.rs index 859953908..55219b59d 100644 --- a/src/sys/windows/mod.rs +++ b/src/sys/windows/mod.rs @@ -14,6 +14,7 @@ use overlapped::Overlapped; mod selector; pub use selector::Selector; +pub(crate) use selector::TokenType; // Macros must be defined before the modules that use them cfg_net! { diff --git a/src/sys/windows/named_pipe.rs b/src/sys/windows/named_pipe.rs index e018d3fde..e46d9aa57 100644 --- a/src/sys/windows/named_pipe.rs +++ b/src/sys/windows/named_pipe.rs @@ -1,8 +1,8 @@ use std::ffi::OsStr; use std::io::{self, Read, Write}; use std::os::windows::io::{AsRawHandle, FromRawHandle, RawHandle}; -use std::sync::atomic::Ordering::{Relaxed, SeqCst}; -use std::sync::atomic::{AtomicBool, AtomicUsize}; +use std::sync::atomic::AtomicBool; +use std::sync::atomic::Ordering::SeqCst; use std::sync::{Arc, Mutex}; use std::{fmt, mem, slice}; @@ -23,9 +23,8 @@ use windows_sys::Win32::System::IO::{ use crate::event::Source; use crate::sys::windows::iocp::{CompletionPort, CompletionStatus}; -use crate::sys::windows::{Event, Handle, Overlapped}; -use crate::Registry; -use crate::{Interest, Token}; +use crate::sys::windows::{Event, Handle, Overlapped, TokenType}; +use crate::{Interest, Registry, Token}; /// Non-blocking windows named pipe. /// @@ -351,9 +350,6 @@ enum State { Err(io::Error), } -// Odd tokens are for named pipes -static NEXT_TOKEN: AtomicUsize = AtomicUsize::new(1); - fn would_block() -> io::Error { io::ErrorKind::WouldBlock.into() } @@ -622,7 +618,7 @@ impl Source for NamedPipe { io.cp = Some(selector.clone_port()); - let inner_token = NEXT_TOKEN.fetch_add(3, Relaxed) + 3; + let inner_token = selector.next_token(TokenType::NamedPipe); selector.inner.cp.add_handle(inner_token, self)?; } diff --git a/src/sys/windows/process.rs b/src/sys/windows/process.rs index 5dc7e8ba4..8625d2cca 100644 --- a/src/sys/windows/process.rs +++ b/src/sys/windows/process.rs @@ -1,26 +1,16 @@ use crate::event::Source; -use crate::sys::windows::iocp::CompletionPort; use crate::{Interest, Registry, Token}; -use std::collections::HashMap; use std::io; use std::mem::size_of; use std::os::windows::io::{ AsHandle, AsRawHandle, BorrowedHandle, FromRawHandle, IntoRawHandle, OwnedHandle, RawHandle, }; -use std::sync::atomic::AtomicUsize; -use std::sync::atomic::Ordering::Relaxed; -use std::sync::{Arc, LazyLock, Mutex}; +use std::sync::Mutex; use windows_sys::Win32::System::JobObjects::{ AssignProcessToJobObject, CreateJobObjectW, JobObjectAssociateCompletionPortInformation, SetInformationJobObject, JOBOBJECT_ASSOCIATE_COMPLETION_PORT, }; -// Odd tokens are for named pipes -static NEXT_TOKEN: AtomicUsize = AtomicUsize::new(2); -// inner token -> token -pub(crate) static TOKEN_MAPPING: LazyLock>> = - LazyLock::new(|| Mutex::new(HashMap::new())); - // https://devblogs.microsoft.com/oldnewthing/20130405-00/?p=4743 #[derive(Debug)] pub struct Process { @@ -32,10 +22,7 @@ impl Process { pub fn new(child: RawHandle) -> io::Result { let job = new_job()?; assign_process_to_job(job.as_raw_handle(), child)?; - let io = Mutex::new(Io { - cp: None, - token: None, - }); + let io = Mutex::new(Io { inner_token: None }); Ok(Self { job, io }) } } @@ -54,10 +41,7 @@ impl AsHandle for Process { impl FromRawHandle for Process { unsafe fn from_raw_handle(job: RawHandle) -> Self { - let io = Mutex::new(Io { - cp: None, - token: None, - }); + let io = Mutex::new(Io { inner_token: None }); let job = OwnedHandle::from_raw_handle(job); Self { job, io } } @@ -78,10 +62,7 @@ impl From for OwnedHandle { impl From for Process { fn from(other: OwnedHandle) -> Self { let job = other.into(); - let io = Mutex::new(Io { - cp: None, - token: None, - }); + let io = Mutex::new(Io { inner_token: None }); Self { job, io } } } @@ -90,87 +71,44 @@ impl Source for Process { fn register(&mut self, registry: &Registry, token: Token, _: Interest) -> io::Result<()> { let mut io = self.io.lock().unwrap(); - io.check_association(registry, false)?; - - if io.token.is_some() { - return Err(io::Error::new( - io::ErrorKind::AlreadyExists, - "I/O source already registered with a `Registry`", - )); + if io.inner_token.is_some() { + return Err(io::ErrorKind::InvalidInput.into()); } - if io.cp.is_none() { - let selector = registry.selector(); - - let port = selector.clone_port(); + let selector = registry.selector(); + let port = selector.as_raw_handle(); + let inner_token = selector.register_process(token)?; + associate_job_with_completion_port(self.job.as_raw_handle(), port, inner_token)?; - let inner_token = NEXT_TOKEN.fetch_add(3, Relaxed) + 3; - TOKEN_MAPPING.lock().unwrap().insert(inner_token, token); - associate_job_with_completion_port( - self.job.as_raw_handle(), - port.as_raw_handle(), - inner_token, - )?; - - io.cp = Some(port); - } + io.inner_token = Some(inner_token); - io.token = Some(token); drop(io); Ok(()) } fn reregister(&mut self, registry: &Registry, token: Token, _: Interest) -> io::Result<()> { - let mut io = self.io.lock().unwrap(); - - io.check_association(registry, true)?; - - io.token = Some(token); - drop(io); - + let io = self.io.lock().unwrap(); + let inner_token = io.inner_token.ok_or_else(|| io::ErrorKind::InvalidInput)?; + registry.selector().reregister_process(inner_token, token)?; Ok(()) } fn deregister(&mut self, registry: &Registry) -> io::Result<()> { let mut io = self.io.lock().unwrap(); - - io.check_association(registry, true)?; - - if io.token.is_none() { - return Err(io::Error::new( - io::ErrorKind::NotFound, - "I/O source not registered with `Registry`", - )); - } - - io.token = None; + let inner_token = io + .inner_token + .take() + .ok_or_else(|| io::ErrorKind::InvalidInput)?; + registry.selector().deregister_process(inner_token)?; Ok(()) } } #[derive(Debug)] struct Io { - // Uniquely identifies the selector associated with this named pipe - cp: Option>, - // Token used to identify events - token: Option, -} - -impl Io { - fn check_association(&self, registry: &Registry, required: bool) -> io::Result<()> { - match self.cp { - Some(ref cp) if !registry.selector().same_port(cp) => Err(io::Error::new( - io::ErrorKind::AlreadyExists, - "I/O source already registered with a different `Registry`", - )), - None if required => Err(io::Error::new( - io::ErrorKind::NotFound, - "I/O source not registered with `Registry`", - )), - _ => Ok(()), - } - } + // Inner token used to identify events + inner_token: Option, } fn new_job() -> io::Result { diff --git a/src/sys/windows/selector.rs b/src/sys/windows/selector.rs index 1e49e421c..b18971621 100644 --- a/src/sys/windows/selector.rs +++ b/src/sys/windows/selector.rs @@ -1,7 +1,6 @@ use super::afd::{self, Afd, AfdPollInfo}; use super::io_status_block::IoStatusBlock; use super::Event; -use crate::sys::windows::process::TOKEN_MAPPING; use crate::sys::Events; use windows_sys::Win32::System::SystemServices::{ JOB_OBJECT_MSG_ABNORMAL_EXIT_PROCESS, JOB_OBJECT_MSG_EXIT_PROCESS, @@ -15,15 +14,13 @@ cfg_net! { } use super::iocp::{CompletionPort, CompletionStatus}; -use std::collections::VecDeque; +use std::collections::{HashMap, VecDeque}; use std::ffi::c_void; use std::io; use std::marker::PhantomPinned; -use std::os::windows::io::RawSocket; +use std::os::windows::io::{AsRawHandle, RawHandle, RawSocket}; use std::pin::Pin; -#[cfg(debug_assertions)] -use std::sync::atomic::AtomicUsize; -use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; use std::sync::{Arc, Mutex}; use std::time::Duration; @@ -57,14 +54,14 @@ cfg_io_source! { const POLL_GROUP_MAX_GROUP_SIZE: usize = 32; impl AfdGroup { - pub fn acquire(&self) -> io::Result> { + pub fn acquire(&self, inner_token: usize) -> io::Result> { let mut afd_group = self.afd_group.lock().unwrap(); if afd_group.len() == 0 { - self._alloc_afd_group(&mut afd_group)?; + self._alloc_afd_group(&mut afd_group, inner_token)?; } else { // + 1 reference in Vec if Arc::strong_count(afd_group.last().unwrap()) > POLL_GROUP_MAX_GROUP_SIZE { - self._alloc_afd_group(&mut afd_group)?; + self._alloc_afd_group(&mut afd_group, inner_token)?; } } @@ -77,8 +74,8 @@ cfg_io_source! { } } - fn _alloc_afd_group(&self, afd_group: &mut Vec>) -> io::Result<()> { - let afd = Afd::new(&self.cp)?; + fn _alloc_afd_group(&self, afd_group: &mut Vec>, inner_token: usize) -> io::Result<()> { + let afd = Afd::new(&self.cp, inner_token)?; let arc = Arc::new(afd); afd_group.push(arc); Ok(()) @@ -367,10 +364,53 @@ impl Selector { self.inner.cp.clone() } - #[cfg(any(feature = "os-ext", feature = "os-proc"))] + pub(super) fn as_raw_handle(&self) -> RawHandle { + self.inner.cp.as_raw_handle() + } + + #[cfg(feature = "os-ext")] pub(super) fn same_port(&self, other: &Arc) -> bool { Arc::ptr_eq(&self.inner.cp, other) } + + pub(super) fn next_token(&self, token_type: TokenType) -> usize { + self.inner.next_token(token_type) + } + + pub(super) fn register_process(&self, token: Token) -> io::Result { + use std::collections::hash_map::Entry::*; + let inner_token = self.inner.next_token(TokenType::Process); + let mut token_mapping = self.inner.token_mapping.lock().unwrap(); + match token_mapping.entry(inner_token) { + Vacant(v) => { + v.insert(token); + } + Occupied(..) => return Err(io::ErrorKind::AlreadyExists.into()), + } + Ok(inner_token) + } + + pub(super) fn reregister_process(&self, inner_token: usize, token: Token) -> io::Result<()> { + use std::collections::hash_map::Entry::*; + let mut token_mapping = self.inner.token_mapping.lock().unwrap(); + match token_mapping.entry(inner_token) { + Vacant(..) => return Err(io::ErrorKind::NotFound.into()), + Occupied(mut o) => { + *o.get_mut() = token; + } + } + Ok(()) + } + + pub(super) fn deregister_process(&self, inner_token: usize) -> io::Result<()> { + self.inner + .token_mapping + .lock() + .unwrap() + .remove(&inner_token) + .ok_or_else(|| io::ErrorKind::NotFound)?; + Ok(()) + } } cfg_io_source! { @@ -403,12 +443,48 @@ cfg_io_source! { } } +/// Inner token type. +/// +/// Token is calculated using the following formula: +/// +/// `next_token * NUM_INNER_TOKEN_TYPES + offset` +/// +/// The `offset` is determined by the token type. `next_token` is the sequence number of the token +/// of a particular type allocated for a particular selector. +#[derive(Debug, PartialEq, Eq)] +#[repr(usize)] +pub enum TokenType { + Afd = 0, + NamedPipe = 1, + Process = 2, +} + +impl From for TokenType { + fn from(other: usize) -> Self { + const AFD: usize = TokenType::Afd as usize; + const NAMED_PIPE: usize = TokenType::NamedPipe as usize; + const PROCESS: usize = TokenType::Process as usize; + match other % NUM_INNER_TOKEN_TYPES { + AFD => TokenType::Afd, + NAMED_PIPE => TokenType::NamedPipe, + PROCESS => TokenType::Process, + _ => unreachable!(), + } + } +} + +const NUM_INNER_TOKEN_TYPES: usize = 3; + #[derive(Debug)] pub struct SelectorInner { pub(super) cp: Arc, update_queue: Mutex>>>>, afd_group: AfdGroup, is_polling: AtomicBool, + next_token: [AtomicUsize; NUM_INNER_TOKEN_TYPES], + // The mapping between inner tokens and public-facing tokens. + // Currently only process tokens are present here. + token_mapping: Mutex>, } // We have ensured thread safety by introducing lock manually. @@ -425,6 +501,14 @@ impl SelectorInner { update_queue: Mutex::new(VecDeque::new()), afd_group: AfdGroup::new(cp_afd), is_polling: AtomicBool::new(false), + // Do not change the order of the items in the following array without changing `TokenType` + // values first. + next_token: [ + AtomicUsize::new(TokenType::Afd as usize), + AtomicUsize::new(TokenType::NamedPipe as usize), + AtomicUsize::new(TokenType::Process as usize), + ], + token_mapping: Mutex::new(Default::default()), } }) } @@ -470,6 +554,11 @@ impl SelectorInner { } } + fn next_token(&self, token_type: TokenType) -> usize { + self.next_token[token_type as usize].fetch_add(NUM_INNER_TOKEN_TYPES, Ordering::Relaxed) + + NUM_INNER_TOKEN_TYPES + } + unsafe fn update_sockets_events(&self) -> io::Result<()> { let mut update_queue = self.update_queue.lock().unwrap(); for sock in update_queue.iter_mut() { @@ -495,14 +584,14 @@ impl SelectorInner { let mut n = 0; let mut update_queue = self.update_queue.lock().unwrap(); for iocp_event in iocp_events.iter() { - if iocp_event.token() % 3 == 2 { - // Process + let token_type: TokenType = iocp_event.token().into(); + if token_type == TokenType::Process { match iocp_event.bytes_transferred() { // We are only interested in "process exit" events to be consistent with `pidfd`. JOB_OBJECT_MSG_EXIT_PROCESS | JOB_OBJECT_MSG_ABNORMAL_EXIT_PROCESS => { // lpOverlapped is the process id - eprintln!("feed_events {}", iocp_event.bytes_transferred()); - if let Some(token) = TOKEN_MAPPING.lock().unwrap().get(&iocp_event.token()) + if let Some(token) = + self.token_mapping.lock().unwrap().get(&iocp_event.token()) { let mut event = Event::new(*token); event.set_readable(); @@ -517,7 +606,7 @@ impl SelectorInner { events.push(Event::from_completion_status(iocp_event)); n += 1; continue; - } else if iocp_event.token() % 3 == 1 { + } else if token_type == TokenType::NamedPipe { // Handle is a named pipe. This could be extended to be any non-AFD event. let callback = (*(iocp_event.overlapped() as *mut super::Overlapped)).callback; @@ -563,7 +652,8 @@ cfg_io_source! { let flags = interests_to_afd_flags(interests); let sock = { - let sock = this._alloc_sock_for_rawsocket(socket)?; + let inner_token = this.next_token(TokenType::Afd); + let sock = this._alloc_sock_for_rawsocket(socket, inner_token)?; let event = Event { flags, data: token.0 as u64, @@ -640,8 +730,9 @@ cfg_io_source! { fn _alloc_sock_for_rawsocket( &self, raw_socket: RawSocket, + inner_token: usize, ) -> io::Result>>> { - let afd = self.afd_group.acquire()?; + let afd = self.afd_group.acquire(inner_token)?; Ok(Arc::pin(Mutex::new(SockState::new(raw_socket, afd)?))) } } @@ -715,18 +806,18 @@ impl Drop for SelectorInner { Ok(iocp_events) => { events_num = iocp_events.iter().len(); for iocp_event in iocp_events.iter() { + let token_type: TokenType = iocp_event.token().into(); if iocp_event.overlapped().is_null() { // Custom event - } else if iocp_event.token() % 3 == 1 { + } else if token_type == TokenType::NamedPipe { // Named pipe, dispatch the event so it can release resources let callback = unsafe { (*(iocp_event.overlapped() as *mut super::Overlapped)).callback }; callback(iocp_event.entry(), None); - } else if iocp_event.token() % 3 == 2 { - // Process - eprintln!("SelectorInner::drop"); + } else if token_type == TokenType::Process { + // The resources are released automatically on drop. } else { // drain sock state to release memory of Arc reference let _sock_state = from_overlapped(iocp_event.overlapped()); diff --git a/tests/util/mod.rs b/tests/util/mod.rs index a899ad31b..33d0df83d 100644 --- a/tests/util/mod.rs +++ b/tests/util/mod.rs @@ -139,7 +139,7 @@ impl From for Readiness { } pub fn expect_events(poll: &mut Poll, events: &mut Events, mut expected: Vec) { - const TIMEOUT: Duration = Duration::from_millis(500); + const TIMEOUT: Duration = Duration::from_millis(4999); const MAX_ITERATIONS: usize = 1000; // In a lot of calls we expect more then one event, but it could be that @@ -150,6 +150,7 @@ pub fn expect_events(poll: &mut Poll, events: &mut Events, mut expected: Vec= TIMEOUT && events.is_empty() { + warn!("poll timed out"); // Poll timed out. break; } From f41a499ac96b780fcbc3d6955465e560fb046208 Mon Sep 17 00:00:00 2001 From: Ivan Gankevich Date: Sat, 28 Dec 2024 08:55:31 +0100 Subject: [PATCH 09/27] Rework how registered handles are managed. --- src/process.rs | 4 +- src/sys/windows/afd.rs | 12 ++- src/sys/windows/mod.rs | 1 - src/sys/windows/named_pipe.rs | 14 ++-- src/sys/windows/process.rs | 49 ++++-------- src/sys/windows/selector.rs | 135 ++++++++++++---------------------- 6 files changed, 78 insertions(+), 137 deletions(-) diff --git a/src/process.rs b/src/process.rs index 94617553e..d881c86a7 100644 --- a/src/process.rs +++ b/src/process.rs @@ -2,7 +2,7 @@ use crate::event::Source; use crate::{sys, Interest, Registry, Token}; use std::io::Error; #[cfg(windows)] -use std::os::windows::io::{AsRawHandle, RawHandle}; +use std::os::windows::io::AsRawHandle; use std::process::Child; /// Process allows polling OS processes for completion. @@ -14,8 +14,6 @@ use std::process::Child; /// Events are delivered even if the process has exited and has been waited for /// by the time [`poll`](crate::Poll::poll) is called. /// -/// On Windows a process might be registered with at most one poller. -/// /// # Implementation notes /// /// [`Process`] uses `pidfd` on Linux, `EVFILT_PROC` on MacOS/BSD and `AssignProcessToJobObject` on diff --git a/src/sys/windows/afd.rs b/src/sys/windows/afd.rs index 412302081..11373cfca 100644 --- a/src/sys/windows/afd.rs +++ b/src/sys/windows/afd.rs @@ -117,6 +117,7 @@ cfg_io_source! { use std::mem::zeroed; use std::os::windows::io::{FromRawHandle, RawHandle}; use std::ptr::null_mut; + use std::sync::atomic::{AtomicUsize, Ordering}; use windows_sys::Wdk::Foundation::OBJECT_ATTRIBUTES; use windows_sys::Wdk::Storage::FileSystem::{NtCreateFile, FILE_OPEN}; @@ -161,6 +162,8 @@ cfg_io_source! { 'o' as _ ]; + static NEXT_TOKEN: AtomicUsize = AtomicUsize::new(0); + impl AfdPollInfo { pub fn zeroed() -> AfdPollInfo { unsafe { zeroed() } @@ -169,7 +172,7 @@ cfg_io_source! { impl Afd { /// Create new Afd instance. - pub(crate) fn new(cp: &CompletionPort, inner_token: usize) -> io::Result { + pub(crate) fn new(cp: &CompletionPort) -> io::Result { let mut afd_helper_handle: HANDLE = INVALID_HANDLE_VALUE; let mut iosb = IO_STATUS_BLOCK { Anonymous: IO_STATUS_BLOCK_0 { Status: 0 }, @@ -198,8 +201,13 @@ cfg_io_source! { return Err(io::Error::new(raw_err.kind(), msg)); } let fd = File::from_raw_handle(afd_helper_handle as RawHandle); + // Increment by 2 to reserve space for other types of handles. + // Non-AFD types (currently only NamedPipe), use odd numbered + // tokens. This allows the selector to differentiate between them + // and dispatch events accordingly. + let token = NEXT_TOKEN.fetch_add(2, Ordering::Relaxed) + 2; let afd = Afd { fd }; - cp.add_handle(inner_token, &afd.fd)?; + cp.add_handle(token, &afd.fd)?; match SetFileCompletionNotificationModes( afd_helper_handle, FILE_SKIP_SET_EVENT_ON_HANDLE as u8 // This is just 2, so fits in u8 diff --git a/src/sys/windows/mod.rs b/src/sys/windows/mod.rs index 55219b59d..859953908 100644 --- a/src/sys/windows/mod.rs +++ b/src/sys/windows/mod.rs @@ -14,7 +14,6 @@ use overlapped::Overlapped; mod selector; pub use selector::Selector; -pub(crate) use selector::TokenType; // Macros must be defined before the modules that use them cfg_net! { diff --git a/src/sys/windows/named_pipe.rs b/src/sys/windows/named_pipe.rs index e46d9aa57..eb35d3797 100644 --- a/src/sys/windows/named_pipe.rs +++ b/src/sys/windows/named_pipe.rs @@ -1,8 +1,8 @@ use std::ffi::OsStr; use std::io::{self, Read, Write}; use std::os::windows::io::{AsRawHandle, FromRawHandle, RawHandle}; -use std::sync::atomic::AtomicBool; -use std::sync::atomic::Ordering::SeqCst; +use std::sync::atomic::Ordering::{Relaxed, SeqCst}; +use std::sync::atomic::{AtomicBool, AtomicUsize}; use std::sync::{Arc, Mutex}; use std::{fmt, mem, slice}; @@ -23,8 +23,9 @@ use windows_sys::Win32::System::IO::{ use crate::event::Source; use crate::sys::windows::iocp::{CompletionPort, CompletionStatus}; -use crate::sys::windows::{Event, Handle, Overlapped, TokenType}; -use crate::{Interest, Registry, Token}; +use crate::sys::windows::{Event, Handle, Overlapped}; +use crate::Registry; +use crate::{Interest, Token}; /// Non-blocking windows named pipe. /// @@ -350,6 +351,9 @@ enum State { Err(io::Error), } +// Odd tokens are for named pipes +static NEXT_TOKEN: AtomicUsize = AtomicUsize::new(1); + fn would_block() -> io::Error { io::ErrorKind::WouldBlock.into() } @@ -618,7 +622,7 @@ impl Source for NamedPipe { io.cp = Some(selector.clone_port()); - let inner_token = selector.next_token(TokenType::NamedPipe); + let inner_token = NEXT_TOKEN.fetch_add(2, Relaxed) + 2; selector.inner.cp.add_handle(inner_token, self)?; } diff --git a/src/sys/windows/process.rs b/src/sys/windows/process.rs index 8625d2cca..ad92e5922 100644 --- a/src/sys/windows/process.rs +++ b/src/sys/windows/process.rs @@ -1,11 +1,11 @@ use crate::event::Source; +use crate::sys::windows::selector::HandleInfo; use crate::{Interest, Registry, Token}; use std::io; use std::mem::size_of; use std::os::windows::io::{ AsHandle, AsRawHandle, BorrowedHandle, FromRawHandle, IntoRawHandle, OwnedHandle, RawHandle, }; -use std::sync::Mutex; use windows_sys::Win32::System::JobObjects::{ AssignProcessToJobObject, CreateJobObjectW, JobObjectAssociateCompletionPortInformation, SetInformationJobObject, JOBOBJECT_ASSOCIATE_COMPLETION_PORT, @@ -15,15 +15,13 @@ use windows_sys::Win32::System::JobObjects::{ #[derive(Debug)] pub struct Process { job: OwnedHandle, - io: Mutex, } impl Process { pub fn new(child: RawHandle) -> io::Result { let job = new_job()?; assign_process_to_job(job.as_raw_handle(), child)?; - let io = Mutex::new(Io { inner_token: None }); - Ok(Self { job, io }) + Ok(Self { job }) } } @@ -41,9 +39,8 @@ impl AsHandle for Process { impl FromRawHandle for Process { unsafe fn from_raw_handle(job: RawHandle) -> Self { - let io = Mutex::new(Io { inner_token: None }); let job = OwnedHandle::from_raw_handle(job); - Self { job, io } + Self { job } } } @@ -62,55 +59,35 @@ impl From for OwnedHandle { impl From for Process { fn from(other: OwnedHandle) -> Self { let job = other.into(); - let io = Mutex::new(Io { inner_token: None }); - Self { job, io } + Self { job } } } impl Source for Process { fn register(&mut self, registry: &Registry, token: Token, _: Interest) -> io::Result<()> { - let mut io = self.io.lock().unwrap(); - - if io.inner_token.is_some() { - return Err(io::ErrorKind::InvalidInput.into()); - } - let selector = registry.selector(); let port = selector.as_raw_handle(); - let inner_token = selector.register_process(token)?; - associate_job_with_completion_port(self.job.as_raw_handle(), port, inner_token)?; - - io.inner_token = Some(inner_token); - - drop(io); - + let handle = self.job.as_raw_handle(); + let info = HandleInfo::Process(token); + selector.register_handle(handle, info)?; + associate_job_with_completion_port(self.job.as_raw_handle(), port, handle as usize)?; Ok(()) } fn reregister(&mut self, registry: &Registry, token: Token, _: Interest) -> io::Result<()> { - let io = self.io.lock().unwrap(); - let inner_token = io.inner_token.ok_or_else(|| io::ErrorKind::InvalidInput)?; - registry.selector().reregister_process(inner_token, token)?; + let handle = self.job.as_raw_handle(); + let info = HandleInfo::Process(token); + registry.selector().reregister_handle(handle, info)?; Ok(()) } fn deregister(&mut self, registry: &Registry) -> io::Result<()> { - let mut io = self.io.lock().unwrap(); - let inner_token = io - .inner_token - .take() - .ok_or_else(|| io::ErrorKind::InvalidInput)?; - registry.selector().deregister_process(inner_token)?; + let handle = self.job.as_raw_handle(); + registry.selector().deregister_handle(handle)?; Ok(()) } } -#[derive(Debug)] -struct Io { - // Inner token used to identify events - inner_token: Option, -} - fn new_job() -> io::Result { // SAFETY: `CreateJobObjectW` returns null pointer on failure and a valid job handle otherwise. let handle = unsafe { CreateJobObjectW(std::ptr::null(), std::ptr::null()) }; diff --git a/src/sys/windows/selector.rs b/src/sys/windows/selector.rs index b18971621..0500d0f8f 100644 --- a/src/sys/windows/selector.rs +++ b/src/sys/windows/selector.rs @@ -20,7 +20,9 @@ use std::io; use std::marker::PhantomPinned; use std::os::windows::io::{AsRawHandle, RawHandle, RawSocket}; use std::pin::Pin; -use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; +#[cfg(debug_assertions)] +use std::sync::atomic::AtomicUsize; +use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Arc, Mutex}; use std::time::Duration; @@ -54,14 +56,14 @@ cfg_io_source! { const POLL_GROUP_MAX_GROUP_SIZE: usize = 32; impl AfdGroup { - pub fn acquire(&self, inner_token: usize) -> io::Result> { + pub fn acquire(&self) -> io::Result> { let mut afd_group = self.afd_group.lock().unwrap(); if afd_group.len() == 0 { - self._alloc_afd_group(&mut afd_group, inner_token)?; + self._alloc_afd_group(&mut afd_group)?; } else { // + 1 reference in Vec if Arc::strong_count(afd_group.last().unwrap()) > POLL_GROUP_MAX_GROUP_SIZE { - self._alloc_afd_group(&mut afd_group, inner_token)?; + self._alloc_afd_group(&mut afd_group)?; } } @@ -74,8 +76,8 @@ cfg_io_source! { } } - fn _alloc_afd_group(&self, afd_group: &mut Vec>, inner_token: usize) -> io::Result<()> { - let afd = Afd::new(&self.cp, inner_token)?; + fn _alloc_afd_group(&self, afd_group: &mut Vec>) -> io::Result<()> { + let afd = Afd::new(&self.cp)?; let arc = Arc::new(afd); afd_group.push(arc); Ok(()) @@ -373,41 +375,36 @@ impl Selector { Arc::ptr_eq(&self.inner.cp, other) } - pub(super) fn next_token(&self, token_type: TokenType) -> usize { - self.inner.next_token(token_type) - } - - pub(super) fn register_process(&self, token: Token) -> io::Result { + pub(super) fn register_handle(&self, handle: RawHandle, info: HandleInfo) -> io::Result<()> { use std::collections::hash_map::Entry::*; - let inner_token = self.inner.next_token(TokenType::Process); - let mut token_mapping = self.inner.token_mapping.lock().unwrap(); - match token_mapping.entry(inner_token) { + let mut handles = self.inner.handles.lock().unwrap(); + match handles.entry(handle) { Vacant(v) => { - v.insert(token); + v.insert(info); } Occupied(..) => return Err(io::ErrorKind::AlreadyExists.into()), } - Ok(inner_token) + Ok(()) } - pub(super) fn reregister_process(&self, inner_token: usize, token: Token) -> io::Result<()> { + pub(super) fn reregister_handle(&self, handle: RawHandle, info: HandleInfo) -> io::Result<()> { use std::collections::hash_map::Entry::*; - let mut token_mapping = self.inner.token_mapping.lock().unwrap(); - match token_mapping.entry(inner_token) { + let mut handles = self.inner.handles.lock().unwrap(); + match handles.entry(handle) { Vacant(..) => return Err(io::ErrorKind::NotFound.into()), Occupied(mut o) => { - *o.get_mut() = token; + *o.get_mut() = info; } } Ok(()) } - pub(super) fn deregister_process(&self, inner_token: usize) -> io::Result<()> { + pub(super) fn deregister_handle(&self, handle: RawHandle) -> io::Result<()> { self.inner - .token_mapping + .handles .lock() .unwrap() - .remove(&inner_token) + .remove(&handle) .ok_or_else(|| io::ErrorKind::NotFound)?; Ok(()) } @@ -443,48 +440,22 @@ cfg_io_source! { } } -/// Inner token type. -/// -/// Token is calculated using the following formula: -/// -/// `next_token * NUM_INNER_TOKEN_TYPES + offset` -/// -/// The `offset` is determined by the token type. `next_token` is the sequence number of the token -/// of a particular type allocated for a particular selector. -#[derive(Debug, PartialEq, Eq)] -#[repr(usize)] -pub enum TokenType { - Afd = 0, - NamedPipe = 1, - Process = 2, -} - -impl From for TokenType { - fn from(other: usize) -> Self { - const AFD: usize = TokenType::Afd as usize; - const NAMED_PIPE: usize = TokenType::NamedPipe as usize; - const PROCESS: usize = TokenType::Process as usize; - match other % NUM_INNER_TOKEN_TYPES { - AFD => TokenType::Afd, - NAMED_PIPE => TokenType::NamedPipe, - PROCESS => TokenType::Process, - _ => unreachable!(), - } - } +/// The data associated with a registered handle. +#[derive(Debug)] +pub enum HandleInfo { + Process(Token), } -const NUM_INNER_TOKEN_TYPES: usize = 3; - #[derive(Debug)] pub struct SelectorInner { pub(super) cp: Arc, update_queue: Mutex>>>>, afd_group: AfdGroup, is_polling: AtomicBool, - next_token: [AtomicUsize; NUM_INNER_TOKEN_TYPES], - // The mapping between inner tokens and public-facing tokens. - // Currently only process tokens are present here. - token_mapping: Mutex>, + /// Raw handles registered in the selector. + /// + /// Currently contains only `Process`-related handles. + handles: Mutex>, } // We have ensured thread safety by introducing lock manually. @@ -501,14 +472,7 @@ impl SelectorInner { update_queue: Mutex::new(VecDeque::new()), afd_group: AfdGroup::new(cp_afd), is_polling: AtomicBool::new(false), - // Do not change the order of the items in the following array without changing `TokenType` - // values first. - next_token: [ - AtomicUsize::new(TokenType::Afd as usize), - AtomicUsize::new(TokenType::NamedPipe as usize), - AtomicUsize::new(TokenType::Process as usize), - ], - token_mapping: Mutex::new(Default::default()), + handles: Mutex::new(Default::default()), } }) } @@ -554,11 +518,6 @@ impl SelectorInner { } } - fn next_token(&self, token_type: TokenType) -> usize { - self.next_token[token_type as usize].fetch_add(NUM_INNER_TOKEN_TYPES, Ordering::Relaxed) - + NUM_INNER_TOKEN_TYPES - } - unsafe fn update_sockets_events(&self) -> io::Result<()> { let mut update_queue = self.update_queue.lock().unwrap(); for sock in update_queue.iter_mut() { @@ -584,20 +543,16 @@ impl SelectorInner { let mut n = 0; let mut update_queue = self.update_queue.lock().unwrap(); for iocp_event in iocp_events.iter() { - let token_type: TokenType = iocp_event.token().into(); - if token_type == TokenType::Process { + let handle = iocp_event.token() as RawHandle; + if let Some(HandleInfo::Process(token)) = self.handles.lock().unwrap().get(&handle) { match iocp_event.bytes_transferred() { // We are only interested in "process exit" events to be consistent with `pidfd`. JOB_OBJECT_MSG_EXIT_PROCESS | JOB_OBJECT_MSG_ABNORMAL_EXIT_PROCESS => { // lpOverlapped is the process id - if let Some(token) = - self.token_mapping.lock().unwrap().get(&iocp_event.token()) - { - let mut event = Event::new(*token); - event.set_readable(); - events.push(event); - n += 1; - } + let mut event = Event::new(*token); + event.set_readable(); + events.push(event); + n += 1; } _ => {} } @@ -606,7 +561,7 @@ impl SelectorInner { events.push(Event::from_completion_status(iocp_event)); n += 1; continue; - } else if token_type == TokenType::NamedPipe { + } else if iocp_event.token() % 2 == 1 { // Handle is a named pipe. This could be extended to be any non-AFD event. let callback = (*(iocp_event.overlapped() as *mut super::Overlapped)).callback; @@ -652,8 +607,7 @@ cfg_io_source! { let flags = interests_to_afd_flags(interests); let sock = { - let inner_token = this.next_token(TokenType::Afd); - let sock = this._alloc_sock_for_rawsocket(socket, inner_token)?; + let sock = this._alloc_sock_for_rawsocket(socket)?; let event = Event { flags, data: token.0 as u64, @@ -730,9 +684,8 @@ cfg_io_source! { fn _alloc_sock_for_rawsocket( &self, raw_socket: RawSocket, - inner_token: usize, ) -> io::Result>>> { - let afd = self.afd_group.acquire(inner_token)?; + let afd = self.afd_group.acquire()?; Ok(Arc::pin(Mutex::new(SockState::new(raw_socket, afd)?))) } } @@ -806,18 +759,20 @@ impl Drop for SelectorInner { Ok(iocp_events) => { events_num = iocp_events.iter().len(); for iocp_event in iocp_events.iter() { - let token_type: TokenType = iocp_event.token().into(); - if iocp_event.overlapped().is_null() { + let handle = iocp_event.token() as RawHandle; + if let Some(HandleInfo::Process(..)) = + self.handles.lock().unwrap().get(&handle) + { + // The resources are released automatically on drop. + } else if iocp_event.overlapped().is_null() { // Custom event - } else if token_type == TokenType::NamedPipe { + } else if iocp_event.token() % 2 == 1 { // Named pipe, dispatch the event so it can release resources let callback = unsafe { (*(iocp_event.overlapped() as *mut super::Overlapped)).callback }; callback(iocp_event.entry(), None); - } else if token_type == TokenType::Process { - // The resources are released automatically on drop. } else { // drain sock state to release memory of Arc reference let _sock_state = from_overlapped(iocp_event.overlapped()); From c6b9a7b367f2ccb31425b3cf8c5853b869e34ea4 Mon Sep 17 00:00:00 2001 From: Ivan Gankevich Date: Sat, 28 Dec 2024 09:27:01 +0100 Subject: [PATCH 10/27] Clean-up. --- src/macros.rs | 10 +++ src/process.rs | 172 ++++++++++++++++++++---------------- src/sys/unix/process/pid.rs | 4 + src/sys/windows/mod.rs | 44 ++++++--- src/sys/windows/process.rs | 84 +++++++----------- 5 files changed, 173 insertions(+), 141 deletions(-) diff --git a/src/macros.rs b/src/macros.rs index 0f147a674..b089a0a78 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -80,6 +80,16 @@ macro_rules! cfg_os_proc { } } +macro_rules! cfg_if { + ($condition:meta, $($item:item)*) => { + $( + #[cfg($condition)] + #[cfg_attr(docsrs, doc(cfg($condition)))] + $item + )* + } +} + macro_rules! trace { ($($t:tt)*) => { log!(trace, $($t)*) diff --git a/src/process.rs b/src/process.rs index d881c86a7..3520a289c 100644 --- a/src/process.rs +++ b/src/process.rs @@ -1,8 +1,6 @@ use crate::event::Source; use crate::{sys, Interest, Registry, Token}; use std::io::Error; -#[cfg(windows)] -use std::os::windows::io::AsRawHandle; use std::process::Child; /// Process allows polling OS processes for completion. @@ -26,7 +24,7 @@ pub struct Process { impl Process { /// Create new process from [`Child`](std::process::Child). pub fn new(child: &Child) -> Result { - let inner = sys::Process::new(child.as_raw_handle())?; + let inner = sys::Process::new(child)?; Ok(Self { inner }) } @@ -63,105 +61,125 @@ impl Source for Process { } // The following trait implementations are useful to send/receive `pidfd` over a UNIX-domain socket. -#[cfg(any( - target_os = "android", - target_os = "espidf", - target_os = "fuchsia", - target_os = "hermit", - target_os = "illumos", - target_os = "linux", -))] -mod linux { - use super::*; - #[cfg(not(target_os = "hermit"))] - use std::os::fd::{AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd, OwnedFd, RawFd}; - // TODO: once is fixed this - // can use `std::os::fd` and be merged with the above. - #[cfg(target_os = "hermit")] - use std::os::hermit::io::{AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd, OwnedFd, RawFd}; - - impl AsFd for Process { - fn as_fd(&self) -> BorrowedFd<'_> { - self.inner.as_fd() +cfg_if! { + any( + target_os = "android", + target_os = "espidf", + target_os = "fuchsia", + target_os = "hermit", + target_os = "illumos", + target_os = "linux", + ), + mod linux { + use super::*; + #[cfg(not(target_os = "hermit"))] + use std::os::fd::{AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd, OwnedFd, RawFd}; + // TODO: once is fixed this + // can use `std::os::fd` and be merged with the above. + #[cfg(target_os = "hermit")] + use std::os::hermit::io::{AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd, OwnedFd, RawFd}; + + impl AsFd for Process { + fn as_fd(&self) -> BorrowedFd<'_> { + self.inner.as_fd() + } } - } - impl AsRawFd for Process { - fn as_raw_fd(&self) -> RawFd { - self.inner.as_raw_fd() + impl AsRawFd for Process { + fn as_raw_fd(&self) -> RawFd { + self.inner.as_raw_fd() + } } - } - impl FromRawFd for Process { - unsafe fn from_raw_fd(fd: RawFd) -> Self { - let inner = sys::Process::from_raw_fd(fd); - Self { inner } + impl FromRawFd for Process { + unsafe fn from_raw_fd(fd: RawFd) -> Self { + let inner = sys::Process::from_raw_fd(fd); + Self { inner } + } } - } - impl IntoRawFd for Process { - fn into_raw_fd(self) -> RawFd { - self.inner.into_raw_fd() + impl IntoRawFd for Process { + fn into_raw_fd(self) -> RawFd { + self.inner.into_raw_fd() + } } - } - impl From for Process { - fn from(other: OwnedFd) -> Self { - let inner = other.into(); - Self { inner } + impl From for Process { + fn from(other: OwnedFd) -> Self { + let inner = other.into(); + Self { inner } + } } - } - impl From for OwnedFd { - fn from(other: Process) -> Self { - other.inner.into() + impl From for OwnedFd { + fn from(other: Process) -> Self { + other.inner.into() + } } } } -#[cfg(windows)] -#[cfg_attr(docsrs, doc(cfg(windows)))] -mod windows { - use super::*; - use std::os::windows::io::{ - AsHandle, AsRawHandle, BorrowedHandle, FromRawHandle, IntoRawHandle, OwnedHandle, RawHandle, - }; - - impl AsRawHandle for Process { - fn as_raw_handle(&self) -> RawHandle { - self.inner.as_raw_handle() +cfg_if! { + any( + target_os = "freebsd", + target_os = "ios", + target_os = "macos", + target_os = "tvos", + target_os = "visionos", + target_os = "watchos", + ), + impl Process { + /// Get process id. + pub fn pid(&self) -> libc::pid_t { + self.inner.pid() } } +} - impl AsHandle for Process { - fn as_handle(&self) -> BorrowedHandle<'_> { - self.inner.as_handle() +cfg_if! { + windows, + mod windows { + use super::*; + use std::os::windows::io::{ + AsHandle, AsRawHandle, BorrowedHandle, FromRawHandle, IntoRawHandle, OwnedHandle, RawHandle, + }; + + impl AsRawHandle for Process { + fn as_raw_handle(&self) -> RawHandle { + self.inner.as_raw_handle() + } } - } - impl FromRawHandle for Process { - unsafe fn from_raw_handle(job: RawHandle) -> Self { - let inner = sys::Process::from_raw_handle(job); - Self { inner } + impl AsHandle for Process { + fn as_handle(&self) -> BorrowedHandle<'_> { + self.inner.as_handle() + } } - } - impl IntoRawHandle for Process { - fn into_raw_handle(self) -> RawHandle { - self.inner.into_raw_handle() + impl FromRawHandle for Process { + unsafe fn from_raw_handle(job: RawHandle) -> Self { + let inner = sys::Process::from_raw_handle(job); + Self { inner } + } } - } - impl From for OwnedHandle { - fn from(other: Process) -> Self { - other.inner.into() + impl IntoRawHandle for Process { + fn into_raw_handle(self) -> RawHandle { + self.inner.into_raw_handle() + } + } + + impl From for OwnedHandle { + fn from(other: Process) -> Self { + other.inner.into() + } } - } - impl From for Process { - fn from(other: OwnedHandle) -> Self { - let inner = other.into(); - Self { inner } + impl From for Process { + fn from(other: OwnedHandle) -> Self { + let inner = other.into(); + Self { inner } + } } } } diff --git a/src/sys/unix/process/pid.rs b/src/sys/unix/process/pid.rs index 913a54680..d22e0b42a 100644 --- a/src/sys/unix/process/pid.rs +++ b/src/sys/unix/process/pid.rs @@ -17,6 +17,10 @@ impl Process { pub fn from_pid(pid: pid_t) -> Result { Ok(Self { pid }) } + + pub fn pid(&self) -> pid_t { + self.pid + } } impl Source for Process { diff --git a/src/sys/windows/mod.rs b/src/sys/windows/mod.rs index 859953908..2234db043 100644 --- a/src/sys/windows/mod.rs +++ b/src/sys/windows/mod.rs @@ -1,3 +1,6 @@ +use std::os::windows::io::AsRawHandle; +use windows_sys::Win32::Foundation::HANDLE; + mod afd; pub mod event; @@ -13,24 +16,26 @@ mod overlapped; use overlapped::Overlapped; mod selector; +pub(crate) use selector::HandleInfo; pub use selector::Selector; +/// Helper macro to execute a system call that returns an `io::Result`. +// +// Macro must be defined before any modules that uses them. +#[cfg(any(feature = "net", feature = "os-proc")) +macro_rules! syscall { + ($fn: ident ( $($arg: expr),* $(,)* ), $err_test: path, $err_value: expr) => {{ + let res = unsafe { $fn($($arg, )*) }; + if $err_test(&res, &$err_value) { + Err(io::Error::last_os_error()) + } else { + Ok(res) + } + }}; +} + // Macros must be defined before the modules that use them cfg_net! { - /// Helper macro to execute a system call that returns an `io::Result`. - // - // Macro must be defined before any modules that uses them. - macro_rules! syscall { - ($fn: ident ( $($arg: expr),* $(,)* ), $err_test: path, $err_value: expr) => {{ - let res = unsafe { $fn($($arg, )*) }; - if $err_test(&res, &$err_value) { - Err(io::Error::last_os_error()) - } else { - Ok(res) - } - }}; - } - mod net; pub(crate) mod tcp; @@ -49,6 +54,17 @@ pub(crate) use waker::Waker; cfg_os_proc! { mod process; pub(crate) use process::Process; + + /// Helper trait to convert `RawHandle` to `HANDLE`. + pub(crate) trait AsHandlePtr { + fn as_handle_ptr(&self) -> HANDLE; + } + + impl AsHandlePtr for T { + fn as_handle_ptr(&self) -> HANDLE { + self.as_raw_handle() as HANDLE + } + } } cfg_io_source! { diff --git a/src/sys/windows/process.rs b/src/sys/windows/process.rs index ad92e5922..c252db3a3 100644 --- a/src/sys/windows/process.rs +++ b/src/sys/windows/process.rs @@ -1,11 +1,14 @@ use crate::event::Source; -use crate::sys::windows::selector::HandleInfo; +use crate::sys::windows::{AsHandlePtr, HandleInfo}; use crate::{Interest, Registry, Token}; +use std::ffi::c_void; use std::io; use std::mem::size_of; use std::os::windows::io::{ AsHandle, AsRawHandle, BorrowedHandle, FromRawHandle, IntoRawHandle, OwnedHandle, RawHandle, }; +use std::process::Child; +use windows_sys::Win32::Foundation::HANDLE; use windows_sys::Win32::System::JobObjects::{ AssignProcessToJobObject, CreateJobObjectW, JobObjectAssociateCompletionPortInformation, SetInformationJobObject, JOBOBJECT_ASSOCIATE_COMPLETION_PORT, @@ -18,9 +21,21 @@ pub struct Process { } impl Process { - pub fn new(child: RawHandle) -> io::Result { - let job = new_job()?; - assign_process_to_job(job.as_raw_handle(), child)?; + pub fn new(child: &Child) -> io::Result { + // SAFETY: `CreateJobObjectW` returns null pointer on failure and a valid job handle otherwise. + let job = syscall!( + CreateJobObjectW(std::ptr::null(), std::ptr::null()), + PartialEq::eq, + 0 + )?; + // SAFETY: `CreateJobObjectW` returns a valid handle on success. + let job = unsafe { OwnedHandle::from_raw_handle(job as RawHandle) }; + // SAFETY: We provide valid `job` and `child` handles. + syscall!( + AssignProcessToJobObject(job.as_handle_ptr(), child.as_handle_ptr()), + PartialEq::eq, + 0 + )?; Ok(Self { job }) } } @@ -66,11 +81,24 @@ impl From for Process { impl Source for Process { fn register(&mut self, registry: &Registry, token: Token, _: Interest) -> io::Result<()> { let selector = registry.selector(); - let port = selector.as_raw_handle(); let handle = self.job.as_raw_handle(); let info = HandleInfo::Process(token); selector.register_handle(handle, info)?; - associate_job_with_completion_port(self.job.as_raw_handle(), port, handle as usize)?; + let job_port = JOBOBJECT_ASSOCIATE_COMPLETION_PORT { + CompletionKey: self.job.as_raw_handle() as *mut c_void, + CompletionPort: selector.as_raw_handle() as HANDLE, + }; + // SAFETY: We provide valid `job` and `port` handles. + syscall!( + SetInformationJobObject( + self.job.as_handle_ptr(), + JobObjectAssociateCompletionPortInformation, + std::ptr::from_ref(&job_port) as *const c_void, + size_of::() as u32, + ), + PartialEq::eq, + 0 + )?; Ok(()) } @@ -87,47 +115,3 @@ impl Source for Process { Ok(()) } } - -fn new_job() -> io::Result { - // SAFETY: `CreateJobObjectW` returns null pointer on failure and a valid job handle otherwise. - let handle = unsafe { CreateJobObjectW(std::ptr::null(), std::ptr::null()) }; - if handle == 0 { - return Err(io::Error::last_os_error()); - } - // SAFETY: `CreateJobObjectW` returns a valid handle on success. - let job = unsafe { OwnedHandle::from_raw_handle(handle as _) }; - Ok(job) -} - -fn assign_process_to_job(job: RawHandle, child: RawHandle) -> io::Result<()> { - if unsafe { AssignProcessToJobObject(job as _, child as _) } == 0 { - Err(io::Error::last_os_error()) - } else { - Ok(()) - } -} - -fn associate_job_with_completion_port( - job: RawHandle, - port: RawHandle, - key: usize, -) -> io::Result<()> { - let job_port = JOBOBJECT_ASSOCIATE_COMPLETION_PORT { - CompletionKey: key as _, - CompletionPort: port as _, - }; - // SAFETY: We provide valid `job` and `port` handles. - if unsafe { - SetInformationJobObject( - job as _, - JobObjectAssociateCompletionPortInformation, - std::ptr::from_ref(&job_port) as _, - size_of::() as u32, - ) - } == 0 - { - Err(io::Error::last_os_error()) - } else { - Ok(()) - } -} From a700c0f5ae034ea2bf4e531da57277d2bf6ea557 Mon Sep 17 00:00:00 2001 From: Ivan Gankevich Date: Sat, 28 Dec 2024 10:24:14 +0100 Subject: [PATCH 11/27] Fix kqueue. --- src/macros.rs | 27 ++++++++++++++++++++++++++- src/process.rs | 21 +++++++++++---------- src/sys/unix/mod.rs | 21 +++++++++++---------- src/sys/unix/selector/kqueue.rs | 3 +-- tests/process.rs | 27 ++++++++++++++++++++++++++- 5 files changed, 75 insertions(+), 24 deletions(-) diff --git a/src/macros.rs b/src/macros.rs index b089a0a78..43bc5941f 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -73,7 +73,32 @@ macro_rules! cfg_any_os_ext { macro_rules! cfg_os_proc { ($($item:item)*) => { $( - #[cfg(feature = "os-proc")] + #[cfg( + all( + feature = "os-proc", + any( + // pidfd + any( + target_os = "android", + target_os = "illumos", + target_os = "linux", + target_os = "redox", + ), + // kqueue + all( + not(mio_unsupported_force_poll_poll), + any( + target_os = "freebsd", + target_os = "ios", + target_os = "macos", + target_os = "tvos", + target_os = "visionos", + target_os = "watchos", + ), + ), + ), + ), + )] #[cfg_attr(docsrs, doc(cfg(feature = "os-proc")))] $item )* diff --git a/src/process.rs b/src/process.rs index 3520a289c..08084bdb1 100644 --- a/src/process.rs +++ b/src/process.rs @@ -64,11 +64,9 @@ impl Source for Process { cfg_if! { any( target_os = "android", - target_os = "espidf", - target_os = "fuchsia", - target_os = "hermit", target_os = "illumos", target_os = "linux", + target_os = "redox", ), mod linux { use super::*; @@ -120,13 +118,16 @@ cfg_if! { } cfg_if! { - any( - target_os = "freebsd", - target_os = "ios", - target_os = "macos", - target_os = "tvos", - target_os = "visionos", - target_os = "watchos", + all( + not(mio_unsupported_force_poll_poll), // `Process` needs kqueue + any( + target_os = "freebsd", + target_os = "ios", + target_os = "macos", + target_os = "tvos", + target_os = "visionos", + target_os = "watchos", + ), ), impl Process { /// Get process id. diff --git a/src/sys/unix/mod.rs b/src/sys/unix/mod.rs index 84f3b6c78..354b56132 100644 --- a/src/sys/unix/mod.rs +++ b/src/sys/unix/mod.rs @@ -109,19 +109,20 @@ cfg_os_poll! { cfg_os_proc! { #[cfg_attr(any( target_os = "android", - target_os = "espidf", - target_os = "fuchsia", - target_os = "hermit", target_os = "illumos", target_os = "linux", + target_os = "redox", ), path = "process/pidfd.rs")] - #[cfg_attr(any( - target_os = "freebsd", - target_os = "ios", - target_os = "macos", - target_os = "tvos", - target_os = "visionos", - target_os = "watchos", + #[cfg_attr(all( + not(mio_unsupported_force_poll_poll), // `Process` needs kqueue + any( + target_os = "freebsd", + target_os = "ios", + target_os = "macos", + target_os = "tvos", + target_os = "visionos", + target_os = "watchos", + ), ), path = "process/pid.rs")] mod process; pub use self::process::Process; diff --git a/src/sys/unix/selector/kqueue.rs b/src/sys/unix/selector/kqueue.rs index b0b71dc44..1169902fb 100644 --- a/src/sys/unix/selector/kqueue.rs +++ b/src/sys/unix/selector/kqueue.rs @@ -348,8 +348,7 @@ fn get_reregister_flags(interest: bool) -> u16 { flags | libc::EV_ADD } else { flags | libc::EV_DELETE - }; - flags + } } struct Changes { diff --git a/tests/process.rs b/tests/process.rs index b938d8da6..eb3c52a4a 100644 --- a/tests/process.rs +++ b/tests/process.rs @@ -1,4 +1,29 @@ -#![cfg(all(feature = "os-poll", feature = "net", feature = "os-proc"))] +#![cfg(all( + feature = "os-poll", + feature = "net", + feature = "os-proc", + any( + // pidfd + any( + target_os = "android", + target_os = "illumos", + target_os = "linux", + target_os = "redox", + ), + // kqueue + all( + not(mio_unsupported_force_poll_poll), + any( + target_os = "freebsd", + target_os = "ios", + target_os = "macos", + target_os = "tvos", + target_os = "visionos", + target_os = "watchos", + ), + ), + ), +))] use mio::{Interest, Process, Token}; use std::process::{Command, Stdio}; From c1a53e216deb698b6e1711f7896ba79836f4e51c Mon Sep 17 00:00:00 2001 From: Ivan Gankevich Date: Sat, 28 Dec 2024 10:50:40 +0100 Subject: [PATCH 12/27] Debug tests on FreeBSD. --- src/sys/unix/selector/kqueue.rs | 34 +++++++++++++++++++-------------- tests/tcp.rs | 6 +++++- tests/tcp_stream.rs | 19 +++++++++++++++++- tests/unix_datagram.rs | 4 ++++ tests/unix_stream.rs | 31 ++++++++++++++++++++---------- 5 files changed, 68 insertions(+), 26 deletions(-) diff --git a/src/sys/unix/selector/kqueue.rs b/src/sys/unix/selector/kqueue.rs index 1169902fb..059e78475 100644 --- a/src/sys/unix/selector/kqueue.rs +++ b/src/sys/unix/selector/kqueue.rs @@ -193,13 +193,16 @@ impl Selector { } // Used by `Waker`. - #[cfg(any( - target_os = "freebsd", - target_os = "ios", - target_os = "macos", - target_os = "tvos", - target_os = "visionos", - target_os = "watchos" + #[cfg(all( + not(mio_unsupported_force_waker_pipe), + any( + target_os = "freebsd", + target_os = "ios", + target_os = "macos", + target_os = "tvos", + target_os = "visionos", + target_os = "watchos" + ), ))] pub fn setup_waker(&self, token: Token) -> io::Result<()> { // First attempt to accept user space notifications. @@ -221,13 +224,16 @@ impl Selector { } // Used by `Waker`. - #[cfg(any( - target_os = "freebsd", - target_os = "ios", - target_os = "macos", - target_os = "tvos", - target_os = "visionos", - target_os = "watchos" + #[cfg(all( + not(mio_unsupported_force_waker_pipe), + any( + target_os = "freebsd", + target_os = "ios", + target_os = "macos", + target_os = "tvos", + target_os = "visionos", + target_os = "watchos" + ), ))] pub fn wake(&self, token: Token) -> io::Result<()> { let mut kevent = kevent!( diff --git a/tests/tcp.rs b/tests/tcp.rs index ca70938dc..243032dec 100644 --- a/tests/tcp.rs +++ b/tests/tcp.rs @@ -536,6 +536,7 @@ fn connection_reset_by_peer() { } #[test] +#[cfg_attr(target_os = "freebsd", ignore = "Doesn't work on FreeBSD.")] fn connect_error() { let (mut poll, mut events) = init_with_poll(); @@ -563,6 +564,7 @@ fn connect_error() { // Without fastopen we would be getting the connection error assert!(event.is_writable() || event.is_error()); // Solaris poll(2) says POLLHUP and POLLOUT are mutually exclusive. + // Doesn't work on FreeBSD either. #[cfg(not(target_os = "solaris"))] assert!(event.is_write_closed()); break 'outer; @@ -695,7 +697,9 @@ fn write_shutdown() { if cfg!(any( target_os = "hurd", target_os = "solaris", - target_os = "nto" + target_os = "nto", + // Not supported when using `poll` as a `kqueue` replacement on FreeBSD. + all(mio_unsupported_force_poll_poll, target_os = "freebsd"), )) { wait!(poll, is_readable, false); } else { diff --git a/tests/tcp_stream.rs b/tests/tcp_stream.rs index 9ff3c39f2..2f3deae9e 100644 --- a/tests/tcp_stream.rs +++ b/tests/tcp_stream.rs @@ -5,7 +5,8 @@ use std::io::{self, IoSlice, IoSliceMut, Read, Write}; use std::net::{self, Shutdown, SocketAddr}; #[cfg(unix)] use std::os::fd::{AsRawFd, FromRawFd, IntoRawFd}; -use std::sync::{mpsc::channel, Arc, Barrier}; +use std::sync::mpsc::channel; +use std::sync::{Arc, Barrier}; use std::thread; use std::time::Duration; @@ -519,6 +520,10 @@ fn no_events_after_deregister() { #[cfg_attr(target_os = "hurd", ignore = "POLLRDHUP isn't supported on GNU/Hurd")] #[cfg_attr(target_os = "solaris", ignore = "POLLRDHUP isn't supported on Solaris")] #[cfg_attr(target_os = "nto", ignore = "POLLRDHUP isn't supported on NTO")] +#[cfg_attr( + all(mio_unsupported_force_poll_poll, target_os = "freebsd"), + ignore = "POLLRDHUP isn't supported when using `poll` as a `kqueue` replacement on FreeBSD." +)] fn tcp_shutdown_client_read_close_event() { let (mut poll, mut events) = init_with_poll(); let barrier = Arc::new(Barrier::new(2)); @@ -562,6 +567,10 @@ fn tcp_shutdown_client_read_close_event() { ), ignore = "fails; client write_closed events are not found" )] +#[cfg_attr( + all(mio_unsupported_force_poll_poll, target_os = "freebsd"), + ignore = "fails when using `poll` as a `kqueue` replacement on FreeBSD" +)] fn tcp_shutdown_client_write_close_event() { let (mut poll, mut events) = init_with_poll(); let barrier = Arc::new(Barrier::new(2)); @@ -596,6 +605,10 @@ fn tcp_shutdown_client_write_close_event() { #[cfg_attr(target_os = "hurd", ignore = "POLLRDHUP isn't supported on GNU/Hurd")] #[cfg_attr(target_os = "solaris", ignore = "POLLRDHUP isn't supported on Solaris")] #[cfg_attr(target_os = "nto", ignore = "POLLRDHUP isn't supported on NTO")] +#[cfg_attr( + all(mio_unsupported_force_poll_poll, target_os = "freebsd"), + ignore = "POLLRDHUP isn't supported when using `poll` as a `kqueue` replacement on FreeBSD." +)] fn tcp_shutdown_server_write_close_event() { let (mut poll, mut events) = init_with_poll(); let barrier = Arc::new(Barrier::new(2)); @@ -629,6 +642,10 @@ fn tcp_shutdown_server_write_close_event() { #[cfg_attr(target_os = "hurd", ignore = "POLLRDHUP isn't supported on GNU/Hurd")] #[cfg_attr(target_os = "solaris", ignore = "POLLRDHUP isn't supported on Solaris")] #[cfg_attr(target_os = "nto", ignore = "POLLRDHUP isn't supported on NTO")] +#[cfg_attr( + all(mio_unsupported_force_poll_poll, target_os = "freebsd"), + ignore = "POLLRDHUP isn't supported when using `poll` as a `kqueue` replacement on FreeBSD." +)] fn tcp_reset_close_event() { let (mut poll, mut events) = init_with_poll(); diff --git a/tests/unix_datagram.rs b/tests/unix_datagram.rs index 31da16c01..c41a4361d 100644 --- a/tests/unix_datagram.rs +++ b/tests/unix_datagram.rs @@ -181,6 +181,10 @@ fn unix_datagram_pair() { #[cfg_attr(target_os = "hurd", ignore = "POLLRDHUP isn't supported on GNU/Hurd")] #[cfg_attr(target_os = "solaris", ignore = "POLLRDHUP isn't supported on Solaris")] #[cfg_attr(target_os = "nto", ignore = "POLLRDHUP isn't supported on NTO")] +#[cfg_attr( + all(mio_unsupported_force_poll_poll, target_os = "freebsd"), + ignore = "POLLRDHUP isn't supported when using `poll` as a `kqueue` replacement on FreeBSD." +)] fn unix_datagram_shutdown() { let (mut poll, mut events) = init_with_poll(); let path1 = temp_file("unix_datagram_shutdown1"); diff --git a/tests/unix_stream.rs b/tests/unix_stream.rs index fda86bbc5..c73f180b0 100644 --- a/tests/unix_stream.rs +++ b/tests/unix_stream.rs @@ -205,6 +205,10 @@ fn unix_stream_peer_addr() { #[cfg_attr(target_os = "hurd", ignore = "POLLRDHUP isn't supported on GNU/Hurd")] #[cfg_attr(target_os = "solaris", ignore = "POLLRDHUP isn't supported on Solaris")] #[cfg_attr(target_os = "nto", ignore = "POLLRDHUP isn't supported on NTO")] +#[cfg_attr( + all(mio_unsupported_force_poll_poll, target_os = "freebsd"), + ignore = "POLLRDHUP isn't supported when using `poll` as a `kqueue` replacement on FreeBSD." +)] fn unix_stream_shutdown_read() { let (mut poll, mut events) = init_with_poll(); let (handle, remote_addr) = new_echo_listener(1, "unix_stream_shutdown_read"); @@ -294,16 +298,19 @@ fn unix_stream_shutdown_write() { stream.shutdown(Shutdown::Write).unwrap(); - #[cfg(any( - target_os = "dragonfly", - target_os = "freebsd", - target_os = "ios", - target_os = "macos", - target_os = "netbsd", - target_os = "openbsd", - target_os = "tvos", - target_os = "visionos", - target_os = "watchos", + #[cfg(all( + not(mio_unsupported_force_poll_poll), + any( + target_os = "dragonfly", + target_os = "freebsd", + target_os = "ios", + target_os = "macos", + target_os = "netbsd", + target_os = "openbsd", + target_os = "tvos", + target_os = "visionos", + target_os = "watchos" + ), ))] expect_events( &mut poll, @@ -396,6 +403,10 @@ fn unix_stream_shutdown_both() { #[cfg_attr(target_os = "hurd", ignore = "POLLRDHUP isn't supported on GNU/Hurd")] #[cfg_attr(target_os = "solaris", ignore = "POLLRDHUP isn't supported on Solaris")] #[cfg_attr(target_os = "nto", ignore = "POLLRDHUP isn't supported on NTO")] +#[cfg_attr( + all(mio_unsupported_force_poll_poll, target_os = "freebsd"), + ignore = "POLLRDHUP isn't supported when using `poll` as a `kqueue` replacement on FreeBSD." +)] fn unix_stream_shutdown_listener_write() { let (mut poll, mut events) = init_with_poll(); let barrier = Arc::new(Barrier::new(2)); From a3053bed186bed5ddf2b3e58a625d826634e8b91 Mon Sep 17 00:00:00 2001 From: Ivan Gankevich Date: Sat, 28 Dec 2024 11:09:02 +0100 Subject: [PATCH 13/27] Fix compilation on windows. --- src/macros.rs | 2 + src/sys/windows/iocp.rs | 4 +- src/sys/windows/mod.rs | 10 ++-- src/sys/windows/named_pipe.rs | 2 +- src/sys/windows/net.rs | 4 +- src/sys/windows/process.rs | 9 ++- src/sys/windows/selector.rs | 102 ++++++++++++++++++++-------------- src/sys/windows/udp.rs | 4 +- tests/util/mod.rs | 4 +- 9 files changed, 80 insertions(+), 61 deletions(-) diff --git a/src/macros.rs b/src/macros.rs index 43bc5941f..25497f626 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -96,6 +96,8 @@ macro_rules! cfg_os_proc { target_os = "watchos", ), ), + // windows + windows, ), ), )] diff --git a/src/sys/windows/iocp.rs b/src/sys/windows/iocp.rs index 0590bb43f..12a10a46c 100644 --- a/src/sys/windows/iocp.rs +++ b/src/sys/windows/iocp.rs @@ -206,7 +206,7 @@ impl CompletionStatus { /// A completion key is a per-handle key that is specified when it is added /// to an I/O completion port via `add_handle` or `add_socket`. pub fn token(&self) -> usize { - self.0.lpCompletionKey as usize + self.0.lpCompletionKey } /// Returns a pointer to the `Overlapped` structure that was specified when @@ -268,6 +268,6 @@ mod tests { } assert_eq!(s[2].bytes_transferred(), 0); assert_eq!(s[2].token(), 0); - assert_eq!(s[2].overlapped(), 0 as *mut _); + assert_eq!(s[2].overlapped(), std::ptr::null_mut()); } } diff --git a/src/sys/windows/mod.rs b/src/sys/windows/mod.rs index 2234db043..0a4da2fb2 100644 --- a/src/sys/windows/mod.rs +++ b/src/sys/windows/mod.rs @@ -1,6 +1,3 @@ -use std::os::windows::io::AsRawHandle; -use windows_sys::Win32::Foundation::HANDLE; - mod afd; pub mod event; @@ -16,13 +13,12 @@ mod overlapped; use overlapped::Overlapped; mod selector; -pub(crate) use selector::HandleInfo; pub use selector::Selector; /// Helper macro to execute a system call that returns an `io::Result`. // // Macro must be defined before any modules that uses them. -#[cfg(any(feature = "net", feature = "os-proc")) +#[cfg(any(feature = "net", feature = "os-proc"))] macro_rules! syscall { ($fn: ident ( $($arg: expr),* $(,)* ), $err_test: path, $err_value: expr) => {{ let res = unsafe { $fn($($arg, )*) }; @@ -54,6 +50,10 @@ pub(crate) use waker::Waker; cfg_os_proc! { mod process; pub(crate) use process::Process; + pub(crate) use selector::HandleInfo; + + use std::os::windows::io::AsRawHandle; + use windows_sys::Win32::Foundation::HANDLE; /// Helper trait to convert `RawHandle` to `HANDLE`. pub(crate) trait AsHandlePtr { diff --git a/src/sys/windows/named_pipe.rs b/src/sys/windows/named_pipe.rs index eb35d3797..6c6e505f5 100644 --- a/src/sys/windows/named_pipe.rs +++ b/src/sys/windows/named_pipe.rs @@ -947,7 +947,7 @@ fn event_done(status: &OVERLAPPED_ENTRY, events: Option<&mut Vec>) { // cleanup. In this case, `events` is `None` and we don't need to track // the event. if let Some(events) = events { - let mut ev = Event::from_completion_status(&status); + let mut ev = Event::from_completion_status(status); // Reverse the `.data` alteration done in `schedule_event`. This // alteration was done so the selector recognized the event as one from // a named pipe. diff --git a/src/sys/windows/net.rs b/src/sys/windows/net.rs index 5cc235335..c2b540560 100644 --- a/src/sys/windows/net.rs +++ b/src/sys/windows/net.rs @@ -75,7 +75,7 @@ pub(crate) fn socket_addr(addr: &SocketAddr) -> (SocketAddrCRepr, i32) { }; let sockaddr_in = SOCKADDR_IN { - sin_family: AF_INET as u16, // 1 + sin_family: AF_INET, // 1 sin_port: addr.port().to_be(), sin_addr, sin_zero: [0; 8], @@ -97,7 +97,7 @@ pub(crate) fn socket_addr(addr: &SocketAddr) -> (SocketAddrCRepr, i32) { }; let sockaddr_in6 = SOCKADDR_IN6 { - sin6_family: AF_INET6 as u16, // 23 + sin6_family: AF_INET6, // 23 sin6_port: addr.port().to_be(), sin6_addr, sin6_flowinfo: addr.flowinfo(), diff --git a/src/sys/windows/process.rs b/src/sys/windows/process.rs index c252db3a3..607d8abc4 100644 --- a/src/sys/windows/process.rs +++ b/src/sys/windows/process.rs @@ -67,13 +67,12 @@ impl IntoRawHandle for Process { impl From for OwnedHandle { fn from(other: Process) -> Self { - other.job.into() + other.job } } impl From for Process { - fn from(other: OwnedHandle) -> Self { - let job = other.into(); + fn from(job: OwnedHandle) -> Self { Self { job } } } @@ -85,7 +84,7 @@ impl Source for Process { let info = HandleInfo::Process(token); selector.register_handle(handle, info)?; let job_port = JOBOBJECT_ASSOCIATE_COMPLETION_PORT { - CompletionKey: self.job.as_raw_handle() as *mut c_void, + CompletionKey: self.job.as_raw_handle(), CompletionPort: selector.as_raw_handle() as HANDLE, }; // SAFETY: We provide valid `job` and `port` handles. @@ -93,7 +92,7 @@ impl Source for Process { SetInformationJobObject( self.job.as_handle_ptr(), JobObjectAssociateCompletionPortInformation, - std::ptr::from_ref(&job_port) as *const c_void, + &job_port as *const JOBOBJECT_ASSOCIATE_COMPLETION_PORT as *const c_void, size_of::() as u32, ), PartialEq::eq, diff --git a/src/sys/windows/selector.rs b/src/sys/windows/selector.rs index 0500d0f8f..ba5bcf53c 100644 --- a/src/sys/windows/selector.rs +++ b/src/sys/windows/selector.rs @@ -2,9 +2,14 @@ use super::afd::{self, Afd, AfdPollInfo}; use super::io_status_block::IoStatusBlock; use super::Event; use crate::sys::Events; -use windows_sys::Win32::System::SystemServices::{ - JOB_OBJECT_MSG_ABNORMAL_EXIT_PROCESS, JOB_OBJECT_MSG_EXIT_PROCESS, -}; + +cfg_os_proc! { + use std::collections::HashMap; + use std::os::windows::io::AsRawHandle; + use windows_sys::Win32::System::SystemServices::{ + JOB_OBJECT_MSG_ABNORMAL_EXIT_PROCESS, JOB_OBJECT_MSG_EXIT_PROCESS, + }; +} cfg_net! { use crate::sys::event::{ @@ -14,11 +19,11 @@ cfg_net! { } use super::iocp::{CompletionPort, CompletionStatus}; -use std::collections::{HashMap, VecDeque}; +use std::collections::VecDeque; use std::ffi::c_void; use std::io; use std::marker::PhantomPinned; -use std::os::windows::io::{AsRawHandle, RawHandle, RawSocket}; +use std::os::windows::io::{RawHandle, RawSocket}; use std::pin::Pin; #[cfg(debug_assertions)] use std::sync::atomic::AtomicUsize; @@ -366,47 +371,51 @@ impl Selector { self.inner.cp.clone() } - pub(super) fn as_raw_handle(&self) -> RawHandle { - self.inner.cp.as_raw_handle() - } - #[cfg(feature = "os-ext")] pub(super) fn same_port(&self, other: &Arc) -> bool { Arc::ptr_eq(&self.inner.cp, other) } +} + +cfg_os_proc! { + impl Selector { + pub(super) fn as_raw_handle(&self) -> RawHandle { + self.inner.cp.as_raw_handle() + } - pub(super) fn register_handle(&self, handle: RawHandle, info: HandleInfo) -> io::Result<()> { - use std::collections::hash_map::Entry::*; - let mut handles = self.inner.handles.lock().unwrap(); - match handles.entry(handle) { - Vacant(v) => { - v.insert(info); + pub(super) fn register_handle(&self, handle: RawHandle, info: HandleInfo) -> io::Result<()> { + use std::collections::hash_map::Entry::*; + let mut handles = self.inner.handles.lock().unwrap(); + match handles.entry(handle) { + Vacant(v) => { + v.insert(info); + } + Occupied(..) => return Err(io::ErrorKind::AlreadyExists.into()), } - Occupied(..) => return Err(io::ErrorKind::AlreadyExists.into()), + Ok(()) } - Ok(()) - } - pub(super) fn reregister_handle(&self, handle: RawHandle, info: HandleInfo) -> io::Result<()> { - use std::collections::hash_map::Entry::*; - let mut handles = self.inner.handles.lock().unwrap(); - match handles.entry(handle) { - Vacant(..) => return Err(io::ErrorKind::NotFound.into()), - Occupied(mut o) => { - *o.get_mut() = info; + pub(super) fn reregister_handle(&self, handle: RawHandle, info: HandleInfo) -> io::Result<()> { + use std::collections::hash_map::Entry::*; + let mut handles = self.inner.handles.lock().unwrap(); + match handles.entry(handle) { + Vacant(..) => return Err(io::ErrorKind::NotFound.into()), + Occupied(mut o) => { + *o.get_mut() = info; + } } + Ok(()) } - Ok(()) - } - pub(super) fn deregister_handle(&self, handle: RawHandle) -> io::Result<()> { - self.inner - .handles - .lock() - .unwrap() - .remove(&handle) - .ok_or_else(|| io::ErrorKind::NotFound)?; - Ok(()) + pub(super) fn deregister_handle(&self, handle: RawHandle) -> io::Result<()> { + self.inner + .handles + .lock() + .unwrap() + .remove(&handle) + .ok_or(io::ErrorKind::NotFound)?; + Ok(()) + } } } @@ -441,9 +450,10 @@ cfg_io_source! { } /// The data associated with a registered handle. +#[cfg(feature = "os-proc")] #[derive(Debug)] pub enum HandleInfo { - Process(Token), + Process(crate::Token), } #[derive(Debug)] @@ -455,11 +465,13 @@ pub struct SelectorInner { /// Raw handles registered in the selector. /// /// Currently contains only `Process`-related handles. + #[cfg(feature = "os-proc")] handles: Mutex>, } // We have ensured thread safety by introducing lock manually. unsafe impl Sync for SelectorInner {} +unsafe impl Send for SelectorInner {} impl SelectorInner { pub fn new() -> io::Result { @@ -472,6 +484,7 @@ impl SelectorInner { update_queue: Mutex::new(VecDeque::new()), afd_group: AfdGroup::new(cp_afd), is_polling: AtomicBool::new(false), + #[cfg(feature = "os-proc")] handles: Mutex::new(Default::default()), } }) @@ -543,8 +556,9 @@ impl SelectorInner { let mut n = 0; let mut update_queue = self.update_queue.lock().unwrap(); for iocp_event in iocp_events.iter() { - let handle = iocp_event.token() as RawHandle; - if let Some(HandleInfo::Process(token)) = self.handles.lock().unwrap().get(&handle) { + let _handle = iocp_event.token() as RawHandle; + #[cfg(feature = "os-proc")] + if let Some(HandleInfo::Process(token)) = self.handles.lock().unwrap().get(&_handle) { match iocp_event.bytes_transferred() { // We are only interested in "process exit" events to be consistent with `pidfd`. JOB_OBJECT_MSG_EXIT_PROCESS | JOB_OBJECT_MSG_ABNORMAL_EXIT_PROCESS => { @@ -557,7 +571,8 @@ impl SelectorInner { _ => {} } continue; - } else if iocp_event.overlapped().is_null() { + } + if iocp_event.overlapped().is_null() { events.push(Event::from_completion_status(iocp_event)); n += 1; continue; @@ -759,12 +774,15 @@ impl Drop for SelectorInner { Ok(iocp_events) => { events_num = iocp_events.iter().len(); for iocp_event in iocp_events.iter() { - let handle = iocp_event.token() as RawHandle; + let _handle = iocp_event.token() as RawHandle; + #[cfg(feature = "os-proc")] if let Some(HandleInfo::Process(..)) = - self.handles.lock().unwrap().get(&handle) + self.handles.lock().unwrap().get(&_handle) { // The resources are released automatically on drop. - } else if iocp_event.overlapped().is_null() { + continue; + } + if iocp_event.overlapped().is_null() { // Custom event } else if iocp_event.token() % 2 == 1 { // Named pipe, dispatch the event so it can release resources diff --git a/src/sys/windows/udp.rs b/src/sys/windows/udp.rs index 87e269fa3..3e975973f 100644 --- a/src/sys/windows/udp.rs +++ b/src/sys/windows/udp.rs @@ -30,8 +30,8 @@ pub(crate) fn only_v6(socket: &net::UdpSocket) -> io::Result { syscall!( getsockopt( socket.as_raw_socket() as usize, - IPPROTO_IPV6 as i32, - IPV6_V6ONLY as i32, + IPPROTO_IPV6, + IPV6_V6ONLY, optval.as_mut_ptr().cast(), &mut optlen, ), diff --git a/tests/util/mod.rs b/tests/util/mod.rs index 33d0df83d..fee97e320 100644 --- a/tests/util/mod.rs +++ b/tests/util/mod.rs @@ -300,8 +300,8 @@ pub fn set_linger_zero(socket: &TcpStream) { let res = unsafe { setsockopt( socket.as_raw_socket() as _, - SOL_SOCKET as i32, - SO_LINGER as i32, + SOL_SOCKET, + SO_LINGER, &mut val as *mut _ as *mut _, size_of::() as _, ) From 8766c8e28f8fd986967344937427101ecd341a16 Mon Sep 17 00:00:00 2001 From: Ivan Gankevich Date: Sat, 28 Dec 2024 11:09:02 +0100 Subject: [PATCH 14/27] Fix compilation on windows. --- tests/tcp.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/tcp.rs b/tests/tcp.rs index 243032dec..65e6a570a 100644 --- a/tests/tcp.rs +++ b/tests/tcp.rs @@ -564,7 +564,6 @@ fn connect_error() { // Without fastopen we would be getting the connection error assert!(event.is_writable() || event.is_error()); // Solaris poll(2) says POLLHUP and POLLOUT are mutually exclusive. - // Doesn't work on FreeBSD either. #[cfg(not(target_os = "solaris"))] assert!(event.is_write_closed()); break 'outer; From d651b0af5b85abbd28c68bb610bd59d78a22a830 Mon Sep 17 00:00:00 2001 From: Ivan Gankevich Date: Sat, 28 Dec 2024 16:44:05 +0100 Subject: [PATCH 15/27] Fix tests on MacOS. --- src/macros.rs | 63 ++++++++++++-- src/process.rs | 151 ++++++++++++++-------------------- src/sys/shell/process.rs | 63 +++++++++++--- src/sys/unix/mod.rs | 17 ---- src/sys/unix/process/pidfd.rs | 8 +- tests/close_on_drop.rs | 19 +++-- tests/registering.rs | 4 + tests/tcp.rs | 5 +- tests/tcp_stream.rs | 4 +- tests/unix_datagram.rs | 4 +- tests/unix_pipe.rs | 12 +++ tests/unix_stream.rs | 8 +- 12 files changed, 212 insertions(+), 146 deletions(-) diff --git a/src/macros.rs b/src/macros.rs index 25497f626..d0311c0a4 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -77,14 +77,14 @@ macro_rules! cfg_os_proc { all( feature = "os-proc", any( - // pidfd + // pidfd (should be the same as in `cfg_os_proc_pidfd` macro) any( target_os = "android", target_os = "illumos", target_os = "linux", target_os = "redox", ), - // kqueue + // kqueue (should be the same as in `cfg_os_proc_kqueue` macro) all( not(mio_unsupported_force_poll_poll), any( @@ -107,13 +107,64 @@ macro_rules! cfg_os_proc { } } -macro_rules! cfg_if { - ($condition:meta, $($item:item)*) => { +/// `os-proc` feature uses `pidfd` implementation. +macro_rules! cfg_os_proc_pidfd { + ($($item:item)*) => { + $( + #[cfg( + any( + target_os = "android", + target_os = "illumos", + target_os = "linux", + target_os = "redox", + ) + )] + $item + )* + }; +} + +/// `os-proc` feature uses `kqueue` implementation. +macro_rules! cfg_os_proc_kqueue { + ($($item:item)*) => { + $( + #[cfg( + all( + // `Process` needs `kqueue`. + not(mio_unsupported_force_poll_poll), + any( + target_os = "freebsd", + target_os = "ios", + target_os = "macos", + target_os = "tvos", + target_os = "visionos", + target_os = "watchos", + ), + ) + )] + $item + )* + }; +} + +/// `os-proc` feature uses `job-object` implementation. +macro_rules! cfg_os_proc_job_object { + ($($item:item)*) => { $( - #[cfg($condition)] - #[cfg_attr(docsrs, doc(cfg($condition)))] + #[cfg(windows)] $item )* + }; +} + +macro_rules! use_fd_traits { + () => { + #[cfg(not(target_os = "hermit"))] + use std::os::fd::{AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd, OwnedFd, RawFd}; + // TODO: once is fixed this + // can use `std::os::fd` and be merged with the above. + #[cfg(target_os = "hermit")] + use std::os::hermit::io::{AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd, OwnedFd, RawFd}; } } diff --git a/src/process.rs b/src/process.rs index 08084bdb1..0fb96a0a2 100644 --- a/src/process.rs +++ b/src/process.rs @@ -14,8 +14,8 @@ use std::process::Child; /// /// # Implementation notes /// -/// [`Process`] uses `pidfd` on Linux, `EVFILT_PROC` on MacOS/BSD and `AssignProcessToJobObject` on -/// Windows. +/// [`Process`] uses `pidfd` on Linux, `kqueue`'s `EVFILT_PROC` on MacOS/BSD and +/// `AssignProcessToJobObject` on Windows. #[derive(Debug)] pub struct Process { inner: sys::Process, @@ -61,74 +61,49 @@ impl Source for Process { } // The following trait implementations are useful to send/receive `pidfd` over a UNIX-domain socket. -cfg_if! { - any( - target_os = "android", - target_os = "illumos", - target_os = "linux", - target_os = "redox", - ), - mod linux { - use super::*; - #[cfg(not(target_os = "hermit"))] - use std::os::fd::{AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd, OwnedFd, RawFd}; - // TODO: once is fixed this - // can use `std::os::fd` and be merged with the above. - #[cfg(target_os = "hermit")] - use std::os::hermit::io::{AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd, OwnedFd, RawFd}; - - impl AsFd for Process { - fn as_fd(&self) -> BorrowedFd<'_> { - self.inner.as_fd() - } +cfg_os_proc_pidfd! { + use_fd_traits!(); + + impl AsFd for Process { + fn as_fd(&self) -> BorrowedFd<'_> { + self.inner.as_fd() } + } - impl AsRawFd for Process { - fn as_raw_fd(&self) -> RawFd { - self.inner.as_raw_fd() - } + impl AsRawFd for Process { + fn as_raw_fd(&self) -> RawFd { + self.inner.as_raw_fd() } + } - impl FromRawFd for Process { - unsafe fn from_raw_fd(fd: RawFd) -> Self { - let inner = sys::Process::from_raw_fd(fd); - Self { inner } - } + impl FromRawFd for Process { + unsafe fn from_raw_fd(fd: RawFd) -> Self { + let inner = sys::Process::from_raw_fd(fd); + Self { inner } } + } - impl IntoRawFd for Process { - fn into_raw_fd(self) -> RawFd { - self.inner.into_raw_fd() - } + impl IntoRawFd for Process { + fn into_raw_fd(self) -> RawFd { + self.inner.into_raw_fd() } + } - impl From for Process { - fn from(other: OwnedFd) -> Self { - let inner = other.into(); - Self { inner } - } + impl From for Process { + fn from(other: OwnedFd) -> Self { + let inner = other.into(); + Self { inner } } + } - impl From for OwnedFd { - fn from(other: Process) -> Self { - other.inner.into() - } + impl From for OwnedFd { + fn from(other: Process) -> Self { + other.inner.into() } } } -cfg_if! { - all( - not(mio_unsupported_force_poll_poll), // `Process` needs kqueue - any( - target_os = "freebsd", - target_os = "ios", - target_os = "macos", - target_os = "tvos", - target_os = "visionos", - target_os = "watchos", - ), - ), +cfg_os_proc_kqueue! { impl Process { /// Get process id. pub fn pid(&self) -> libc::pid_t { @@ -137,50 +112,46 @@ cfg_if! { } } -cfg_if! { - windows, - mod windows { - use super::*; - use std::os::windows::io::{ - AsHandle, AsRawHandle, BorrowedHandle, FromRawHandle, IntoRawHandle, OwnedHandle, RawHandle, - }; - - impl AsRawHandle for Process { - fn as_raw_handle(&self) -> RawHandle { - self.inner.as_raw_handle() - } +cfg_os_proc_job_object! { + use std::os::windows::io::{ + AsHandle, AsRawHandle, BorrowedHandle, FromRawHandle, IntoRawHandle, OwnedHandle, RawHandle, + }; + + impl AsRawHandle for Process { + fn as_raw_handle(&self) -> RawHandle { + self.inner.as_raw_handle() } + } - impl AsHandle for Process { - fn as_handle(&self) -> BorrowedHandle<'_> { - self.inner.as_handle() - } + impl AsHandle for Process { + fn as_handle(&self) -> BorrowedHandle<'_> { + self.inner.as_handle() } + } - impl FromRawHandle for Process { - unsafe fn from_raw_handle(job: RawHandle) -> Self { - let inner = sys::Process::from_raw_handle(job); - Self { inner } - } + impl FromRawHandle for Process { + unsafe fn from_raw_handle(job: RawHandle) -> Self { + let inner = sys::Process::from_raw_handle(job); + Self { inner } } + } - impl IntoRawHandle for Process { - fn into_raw_handle(self) -> RawHandle { - self.inner.into_raw_handle() - } + impl IntoRawHandle for Process { + fn into_raw_handle(self) -> RawHandle { + self.inner.into_raw_handle() } + } - impl From for OwnedHandle { - fn from(other: Process) -> Self { - other.inner.into() - } + impl From for OwnedHandle { + fn from(other: Process) -> Self { + other.inner.into() } + } - impl From for Process { - fn from(other: OwnedHandle) -> Self { - let inner = other.into(); - Self { inner } - } + impl From for Process { + fn from(other: OwnedHandle) -> Self { + let inner = other.into(); + Self { inner } } } } diff --git a/src/sys/shell/process.rs b/src/sys/shell/process.rs index b84d36d86..54bdf2fcc 100644 --- a/src/sys/shell/process.rs +++ b/src/sys/shell/process.rs @@ -18,18 +18,8 @@ impl Process { } } -#[cfg(any( - target_os = "android", - target_os = "espidf", - target_os = "fuchsia", - target_os = "hermit", - target_os = "illumos", - target_os = "linux", -))] -mod linux { - use std::os::fd::{AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd, OwnedFd, RawFd}; - - use super::*; +cfg_os_proc_pidfd! { + use_fd_traits!(); impl AsFd for Process { fn as_fd(&self) -> BorrowedFd<'_> { @@ -68,6 +58,55 @@ mod linux { } } +cfg_os_proc_kqueue! { + impl Process { + pub fn pid(&self) -> libc::pid_t { + os_required!() + } + } +} + +cfg_os_proc_job_object! { + use std::os::windows::io::{ + AsHandle, AsRawHandle, BorrowedHandle, FromRawHandle, IntoRawHandle, OwnedHandle, RawHandle, + }; + + impl AsRawHandle for Process { + fn as_raw_handle(&self) -> RawHandle { + os_required!() + } + } + + impl AsHandle for Process { + fn as_handle(&self) -> BorrowedHandle<'_> { + os_required!() + } + } + + impl FromRawHandle for Process { + unsafe fn from_raw_handle(_: RawHandle) -> Self { + os_required!() + } + } + + impl IntoRawHandle for Process { + fn into_raw_handle(self) -> RawHandle { + os_required!() + } + } + + impl From for OwnedHandle { + fn from(_: Process) -> Self { + os_required!() + } + } + + impl From for Process { + fn from(_: OwnedHandle) -> Self { + os_required!() + } + } +} impl Source for Process { fn register(&mut self, _: &Registry, _: Token, _: Interest) -> Result<(), Error> { os_required!() diff --git a/src/sys/unix/mod.rs b/src/sys/unix/mod.rs index 354b56132..88670e7aa 100644 --- a/src/sys/unix/mod.rs +++ b/src/sys/unix/mod.rs @@ -107,23 +107,6 @@ cfg_os_poll! { // `poll(2)` implementation needs to do some special stuff. cfg_os_proc! { - #[cfg_attr(any( - target_os = "android", - target_os = "illumos", - target_os = "linux", - target_os = "redox", - ), path = "process/pidfd.rs")] - #[cfg_attr(all( - not(mio_unsupported_force_poll_poll), // `Process` needs kqueue - any( - target_os = "freebsd", - target_os = "ios", - target_os = "macos", - target_os = "tvos", - target_os = "visionos", - target_os = "watchos", - ), - ), path = "process/pid.rs")] mod process; pub use self::process::Process; } diff --git a/src/sys/unix/process/pidfd.rs b/src/sys/unix/process/pidfd.rs index 7907470d0..09cc709f8 100644 --- a/src/sys/unix/process/pidfd.rs +++ b/src/sys/unix/process/pidfd.rs @@ -3,14 +3,10 @@ use crate::{Interest, Registry, Token}; use libc::{pid_t, SYS_pidfd_open, PIDFD_NONBLOCK}; use std::fs::File; use std::io::Error; -#[cfg(not(target_os = "hermit"))] -use std::os::fd::{AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd, OwnedFd, RawFd}; -// TODO: once is fixed this -// can use `std::os::fd` and be merged with the above. -#[cfg(target_os = "hermit")] -use std::os::hermit::io::{AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd, OwnedFd, RawFd}; use std::process::Child; +use_fd_traits!(); + #[derive(Debug)] pub struct Process { fd: File, diff --git a/tests/close_on_drop.rs b/tests/close_on_drop.rs index 058761a89..a2be81114 100644 --- a/tests/close_on_drop.rs +++ b/tests/close_on_drop.rs @@ -4,6 +4,7 @@ use std::io::Read; use log::debug; +use mio::event::Event; use mio::net::{TcpListener, TcpStream}; use mio::{Events, Interest, Poll, Registry, Token}; @@ -38,10 +39,10 @@ impl TestHandler { } } - fn handle_read(&mut self, registry: &Registry, tok: Token) { - debug!("readable; tok={:?}", tok); + fn handle_read(&mut self, registry: &Registry, event: &Event) { + debug!("readable; tok={:?}", event.token()); - match tok { + match event.token() { SERVER => { debug!("server connection ready for accept"); let _ = self.srv.accept().unwrap(); @@ -64,12 +65,14 @@ impl TestHandler { Ok(_) => panic!("the client socket should not be readable"), Err(e) => panic!("Unexpected error {:?}", e), } + if !event.is_read_closed() { + registry + .reregister(&mut self.cli, CLIENT, Interest::READABLE) + .unwrap(); + } } - _ => panic!("received unknown token {:?}", tok), + _ => panic!("received unknown token {:?}", event.token()), } - registry - .reregister(&mut self.cli, CLIENT, Interest::READABLE) - .unwrap(); } fn handle_write(&mut self, registry: &Registry, tok: Token) { @@ -119,7 +122,7 @@ pub fn close_on_drop() { for event in &events { if event.is_readable() { - handler.handle_read(poll.registry(), event.token()); + handler.handle_read(poll.registry(), event); } if event.is_writable() { diff --git a/tests/registering.rs b/tests/registering.rs index c8415b831..987789b7c 100644 --- a/tests/registering.rs +++ b/tests/registering.rs @@ -70,6 +70,10 @@ impl TestHandler { } #[test] +#[cfg_attr( + all(mio_unsupported_force_poll_poll, target_os = "macos"), + ignore = "Doesn't work on MacOS with `poll`." +)] pub fn register_deregister() { init(); diff --git a/tests/tcp.rs b/tests/tcp.rs index 65e6a570a..e1db2cf90 100644 --- a/tests/tcp.rs +++ b/tests/tcp.rs @@ -536,7 +536,10 @@ fn connection_reset_by_peer() { } #[test] -#[cfg_attr(target_os = "freebsd", ignore = "Doesn't work on FreeBSD.")] +#[cfg_attr( + all(mio_unsupported_force_poll_poll, any(target_os = "freebsd", target_os = "macos")), + ignore = "Doesn't work on MacOS/FreeBSD with `poll`." +)] fn connect_error() { let (mut poll, mut events) = init_with_poll(); diff --git a/tests/tcp_stream.rs b/tests/tcp_stream.rs index 2f3deae9e..2585710dd 100644 --- a/tests/tcp_stream.rs +++ b/tests/tcp_stream.rs @@ -568,8 +568,8 @@ fn tcp_shutdown_client_read_close_event() { ignore = "fails; client write_closed events are not found" )] #[cfg_attr( - all(mio_unsupported_force_poll_poll, target_os = "freebsd"), - ignore = "fails when using `poll` as a `kqueue` replacement on FreeBSD" + all(mio_unsupported_force_poll_poll, any(target_os = "freebsd", target_os = "macos")), + ignore = "Doesn't work on MacOS/FreeBSD with `poll`." )] fn tcp_shutdown_client_write_close_event() { let (mut poll, mut events) = init_with_poll(); diff --git a/tests/unix_datagram.rs b/tests/unix_datagram.rs index c41a4361d..9bd38eee8 100644 --- a/tests/unix_datagram.rs +++ b/tests/unix_datagram.rs @@ -182,8 +182,8 @@ fn unix_datagram_pair() { #[cfg_attr(target_os = "solaris", ignore = "POLLRDHUP isn't supported on Solaris")] #[cfg_attr(target_os = "nto", ignore = "POLLRDHUP isn't supported on NTO")] #[cfg_attr( - all(mio_unsupported_force_poll_poll, target_os = "freebsd"), - ignore = "POLLRDHUP isn't supported when using `poll` as a `kqueue` replacement on FreeBSD." + all(mio_unsupported_force_poll_poll, any(target_os = "freebsd", target_os = "macos")), + ignore = "POLLRDHUP isn't supported when using `poll` as a `kqueue` replacement on MacOS/FreeBSD." )] fn unix_datagram_shutdown() { let (mut poll, mut events) = init_with_poll(); diff --git a/tests/unix_pipe.rs b/tests/unix_pipe.rs index 73c28419a..323934140 100644 --- a/tests/unix_pipe.rs +++ b/tests/unix_pipe.rs @@ -57,6 +57,10 @@ fn smoke() { any(target_os = "hurd", target_os = "nto"), ignore = "Writer fd close events do not trigger POLLHUP on nto and GNU/Hurd targets" )] +#[cfg_attr( + all(mio_unsupported_force_poll_poll, any(target_os = "macos")), + ignore = "Doesn't work on MacOS with `poll`." +)] fn event_when_sender_is_dropped() { let mut poll = Poll::new().unwrap(); let mut events = Events::with_capacity(8); @@ -99,6 +103,10 @@ fn event_when_sender_is_dropped() { any(target_os = "hurd", target_os = "nto"), ignore = "Writer fd close events do not trigger POLLHUP on nto and GNU/Hurd targets" )] +#[cfg_attr( + all(mio_unsupported_force_poll_poll, any(target_os = "macos")), + ignore = "Doesn't work on MacOS with `poll`." +)] fn event_when_receiver_is_dropped() { let mut poll = Poll::new().unwrap(); let mut events = Events::with_capacity(8); @@ -136,6 +144,10 @@ fn event_when_receiver_is_dropped() { any(target_os = "hurd", target_os = "nto"), ignore = "Writer fd close events do not trigger POLLHUP on nto and GNU/Hurd targets" )] +#[cfg_attr( + all(mio_unsupported_force_poll_poll, any(target_os = "macos")), + ignore = "Doesn't work on MacOS with `poll`." +)] fn from_child_process_io() { // `cat` simply echo everything that we write via standard in. let mut child = Command::new("cat") diff --git a/tests/unix_stream.rs b/tests/unix_stream.rs index c73f180b0..51ce37742 100644 --- a/tests/unix_stream.rs +++ b/tests/unix_stream.rs @@ -206,8 +206,8 @@ fn unix_stream_peer_addr() { #[cfg_attr(target_os = "solaris", ignore = "POLLRDHUP isn't supported on Solaris")] #[cfg_attr(target_os = "nto", ignore = "POLLRDHUP isn't supported on NTO")] #[cfg_attr( - all(mio_unsupported_force_poll_poll, target_os = "freebsd"), - ignore = "POLLRDHUP isn't supported when using `poll` as a `kqueue` replacement on FreeBSD." + all(mio_unsupported_force_poll_poll, any(target_os = "freebsd", target_os = "macos")), + ignore = "POLLRDHUP isn't supported when using `poll` as a `kqueue` replacement on FreeBSD/MacOS." )] fn unix_stream_shutdown_read() { let (mut poll, mut events) = init_with_poll(); @@ -335,6 +335,10 @@ fn unix_stream_shutdown_write() { target_os = "hurd", ignore = "getting pathname isn't supported on GNU/Hurd" )] +#[cfg_attr( + all(mio_unsupported_force_poll_poll, target_os = "macos"), + ignore = "Doesn't work on MacOS with `poll`." +)] fn unix_stream_shutdown_both() { let (mut poll, mut events) = init_with_poll(); let (handle, remote_addr) = new_echo_listener(1, "unix_stream_shutdown_both"); From 858f8ec4260a18be9a488250df4046f679d9061f Mon Sep 17 00:00:00 2001 From: Ivan Gankevich Date: Sat, 28 Dec 2024 16:55:14 +0100 Subject: [PATCH 16/27] Cleanup --- src/sys/unix/process/mod.rs | 9 +++++ src/sys/unix/selector/kqueue.rs | 34 ++++++++---------- src/sys/windows/selector.rs | 63 ++++++++++++++++----------------- tests/close_on_drop.rs | 19 +++++----- tests/registering.rs | 4 --- tests/tcp.rs | 8 +---- tests/tcp_stream.rs | 19 +--------- tests/unix_datagram.rs | 4 --- tests/unix_pipe.rs | 12 ------- tests/unix_stream.rs | 35 ++++++------------ 10 files changed, 74 insertions(+), 133 deletions(-) create mode 100644 src/sys/unix/process/mod.rs diff --git a/src/sys/unix/process/mod.rs b/src/sys/unix/process/mod.rs new file mode 100644 index 000000000..c15bd56dd --- /dev/null +++ b/src/sys/unix/process/mod.rs @@ -0,0 +1,9 @@ +cfg_os_proc_pidfd! { + mod pidfd; + pub use self::pidfd::*; +} + +cfg_os_proc_kqueue! { + mod pid; + pub use self::pid::*; +} diff --git a/src/sys/unix/selector/kqueue.rs b/src/sys/unix/selector/kqueue.rs index 059e78475..1169902fb 100644 --- a/src/sys/unix/selector/kqueue.rs +++ b/src/sys/unix/selector/kqueue.rs @@ -193,16 +193,13 @@ impl Selector { } // Used by `Waker`. - #[cfg(all( - not(mio_unsupported_force_waker_pipe), - any( - target_os = "freebsd", - target_os = "ios", - target_os = "macos", - target_os = "tvos", - target_os = "visionos", - target_os = "watchos" - ), + #[cfg(any( + target_os = "freebsd", + target_os = "ios", + target_os = "macos", + target_os = "tvos", + target_os = "visionos", + target_os = "watchos" ))] pub fn setup_waker(&self, token: Token) -> io::Result<()> { // First attempt to accept user space notifications. @@ -224,16 +221,13 @@ impl Selector { } // Used by `Waker`. - #[cfg(all( - not(mio_unsupported_force_waker_pipe), - any( - target_os = "freebsd", - target_os = "ios", - target_os = "macos", - target_os = "tvos", - target_os = "visionos", - target_os = "watchos" - ), + #[cfg(any( + target_os = "freebsd", + target_os = "ios", + target_os = "macos", + target_os = "tvos", + target_os = "visionos", + target_os = "watchos" ))] pub fn wake(&self, token: Token) -> io::Result<()> { let mut kevent = kevent!( diff --git a/src/sys/windows/selector.rs b/src/sys/windows/selector.rs index ba5bcf53c..c84a90983 100644 --- a/src/sys/windows/selector.rs +++ b/src/sys/windows/selector.rs @@ -377,45 +377,44 @@ impl Selector { } } -cfg_os_proc! { - impl Selector { - pub(super) fn as_raw_handle(&self) -> RawHandle { - self.inner.cp.as_raw_handle() - } +#[cfg(feature = "os-proc")] +impl Selector { + pub(super) fn as_raw_handle(&self) -> RawHandle { + self.inner.cp.as_raw_handle() + } - pub(super) fn register_handle(&self, handle: RawHandle, info: HandleInfo) -> io::Result<()> { - use std::collections::hash_map::Entry::*; - let mut handles = self.inner.handles.lock().unwrap(); - match handles.entry(handle) { - Vacant(v) => { - v.insert(info); - } - Occupied(..) => return Err(io::ErrorKind::AlreadyExists.into()), + pub(super) fn register_handle(&self, handle: RawHandle, info: HandleInfo) -> io::Result<()> { + use std::collections::hash_map::Entry::*; + let mut handles = self.inner.handles.lock().unwrap(); + match handles.entry(handle) { + Vacant(v) => { + v.insert(info); } - Ok(()) + Occupied(..) => return Err(io::ErrorKind::AlreadyExists.into()), } + Ok(()) + } - pub(super) fn reregister_handle(&self, handle: RawHandle, info: HandleInfo) -> io::Result<()> { - use std::collections::hash_map::Entry::*; - let mut handles = self.inner.handles.lock().unwrap(); - match handles.entry(handle) { - Vacant(..) => return Err(io::ErrorKind::NotFound.into()), - Occupied(mut o) => { - *o.get_mut() = info; - } + pub(super) fn reregister_handle(&self, handle: RawHandle, info: HandleInfo) -> io::Result<()> { + use std::collections::hash_map::Entry::*; + let mut handles = self.inner.handles.lock().unwrap(); + match handles.entry(handle) { + Vacant(..) => return Err(io::ErrorKind::NotFound.into()), + Occupied(mut o) => { + *o.get_mut() = info; } - Ok(()) } + Ok(()) + } - pub(super) fn deregister_handle(&self, handle: RawHandle) -> io::Result<()> { - self.inner - .handles - .lock() - .unwrap() - .remove(&handle) - .ok_or(io::ErrorKind::NotFound)?; - Ok(()) - } + pub(super) fn deregister_handle(&self, handle: RawHandle) -> io::Result<()> { + self.inner + .handles + .lock() + .unwrap() + .remove(&handle) + .ok_or(io::ErrorKind::NotFound)?; + Ok(()) } } diff --git a/tests/close_on_drop.rs b/tests/close_on_drop.rs index a2be81114..058761a89 100644 --- a/tests/close_on_drop.rs +++ b/tests/close_on_drop.rs @@ -4,7 +4,6 @@ use std::io::Read; use log::debug; -use mio::event::Event; use mio::net::{TcpListener, TcpStream}; use mio::{Events, Interest, Poll, Registry, Token}; @@ -39,10 +38,10 @@ impl TestHandler { } } - fn handle_read(&mut self, registry: &Registry, event: &Event) { - debug!("readable; tok={:?}", event.token()); + fn handle_read(&mut self, registry: &Registry, tok: Token) { + debug!("readable; tok={:?}", tok); - match event.token() { + match tok { SERVER => { debug!("server connection ready for accept"); let _ = self.srv.accept().unwrap(); @@ -65,14 +64,12 @@ impl TestHandler { Ok(_) => panic!("the client socket should not be readable"), Err(e) => panic!("Unexpected error {:?}", e), } - if !event.is_read_closed() { - registry - .reregister(&mut self.cli, CLIENT, Interest::READABLE) - .unwrap(); - } } - _ => panic!("received unknown token {:?}", event.token()), + _ => panic!("received unknown token {:?}", tok), } + registry + .reregister(&mut self.cli, CLIENT, Interest::READABLE) + .unwrap(); } fn handle_write(&mut self, registry: &Registry, tok: Token) { @@ -122,7 +119,7 @@ pub fn close_on_drop() { for event in &events { if event.is_readable() { - handler.handle_read(poll.registry(), event); + handler.handle_read(poll.registry(), event.token()); } if event.is_writable() { diff --git a/tests/registering.rs b/tests/registering.rs index 987789b7c..c8415b831 100644 --- a/tests/registering.rs +++ b/tests/registering.rs @@ -70,10 +70,6 @@ impl TestHandler { } #[test] -#[cfg_attr( - all(mio_unsupported_force_poll_poll, target_os = "macos"), - ignore = "Doesn't work on MacOS with `poll`." -)] pub fn register_deregister() { init(); diff --git a/tests/tcp.rs b/tests/tcp.rs index e1db2cf90..ca70938dc 100644 --- a/tests/tcp.rs +++ b/tests/tcp.rs @@ -536,10 +536,6 @@ fn connection_reset_by_peer() { } #[test] -#[cfg_attr( - all(mio_unsupported_force_poll_poll, any(target_os = "freebsd", target_os = "macos")), - ignore = "Doesn't work on MacOS/FreeBSD with `poll`." -)] fn connect_error() { let (mut poll, mut events) = init_with_poll(); @@ -699,9 +695,7 @@ fn write_shutdown() { if cfg!(any( target_os = "hurd", target_os = "solaris", - target_os = "nto", - // Not supported when using `poll` as a `kqueue` replacement on FreeBSD. - all(mio_unsupported_force_poll_poll, target_os = "freebsd"), + target_os = "nto" )) { wait!(poll, is_readable, false); } else { diff --git a/tests/tcp_stream.rs b/tests/tcp_stream.rs index 2585710dd..9ff3c39f2 100644 --- a/tests/tcp_stream.rs +++ b/tests/tcp_stream.rs @@ -5,8 +5,7 @@ use std::io::{self, IoSlice, IoSliceMut, Read, Write}; use std::net::{self, Shutdown, SocketAddr}; #[cfg(unix)] use std::os::fd::{AsRawFd, FromRawFd, IntoRawFd}; -use std::sync::mpsc::channel; -use std::sync::{Arc, Barrier}; +use std::sync::{mpsc::channel, Arc, Barrier}; use std::thread; use std::time::Duration; @@ -520,10 +519,6 @@ fn no_events_after_deregister() { #[cfg_attr(target_os = "hurd", ignore = "POLLRDHUP isn't supported on GNU/Hurd")] #[cfg_attr(target_os = "solaris", ignore = "POLLRDHUP isn't supported on Solaris")] #[cfg_attr(target_os = "nto", ignore = "POLLRDHUP isn't supported on NTO")] -#[cfg_attr( - all(mio_unsupported_force_poll_poll, target_os = "freebsd"), - ignore = "POLLRDHUP isn't supported when using `poll` as a `kqueue` replacement on FreeBSD." -)] fn tcp_shutdown_client_read_close_event() { let (mut poll, mut events) = init_with_poll(); let barrier = Arc::new(Barrier::new(2)); @@ -567,10 +562,6 @@ fn tcp_shutdown_client_read_close_event() { ), ignore = "fails; client write_closed events are not found" )] -#[cfg_attr( - all(mio_unsupported_force_poll_poll, any(target_os = "freebsd", target_os = "macos")), - ignore = "Doesn't work on MacOS/FreeBSD with `poll`." -)] fn tcp_shutdown_client_write_close_event() { let (mut poll, mut events) = init_with_poll(); let barrier = Arc::new(Barrier::new(2)); @@ -605,10 +596,6 @@ fn tcp_shutdown_client_write_close_event() { #[cfg_attr(target_os = "hurd", ignore = "POLLRDHUP isn't supported on GNU/Hurd")] #[cfg_attr(target_os = "solaris", ignore = "POLLRDHUP isn't supported on Solaris")] #[cfg_attr(target_os = "nto", ignore = "POLLRDHUP isn't supported on NTO")] -#[cfg_attr( - all(mio_unsupported_force_poll_poll, target_os = "freebsd"), - ignore = "POLLRDHUP isn't supported when using `poll` as a `kqueue` replacement on FreeBSD." -)] fn tcp_shutdown_server_write_close_event() { let (mut poll, mut events) = init_with_poll(); let barrier = Arc::new(Barrier::new(2)); @@ -642,10 +629,6 @@ fn tcp_shutdown_server_write_close_event() { #[cfg_attr(target_os = "hurd", ignore = "POLLRDHUP isn't supported on GNU/Hurd")] #[cfg_attr(target_os = "solaris", ignore = "POLLRDHUP isn't supported on Solaris")] #[cfg_attr(target_os = "nto", ignore = "POLLRDHUP isn't supported on NTO")] -#[cfg_attr( - all(mio_unsupported_force_poll_poll, target_os = "freebsd"), - ignore = "POLLRDHUP isn't supported when using `poll` as a `kqueue` replacement on FreeBSD." -)] fn tcp_reset_close_event() { let (mut poll, mut events) = init_with_poll(); diff --git a/tests/unix_datagram.rs b/tests/unix_datagram.rs index 9bd38eee8..31da16c01 100644 --- a/tests/unix_datagram.rs +++ b/tests/unix_datagram.rs @@ -181,10 +181,6 @@ fn unix_datagram_pair() { #[cfg_attr(target_os = "hurd", ignore = "POLLRDHUP isn't supported on GNU/Hurd")] #[cfg_attr(target_os = "solaris", ignore = "POLLRDHUP isn't supported on Solaris")] #[cfg_attr(target_os = "nto", ignore = "POLLRDHUP isn't supported on NTO")] -#[cfg_attr( - all(mio_unsupported_force_poll_poll, any(target_os = "freebsd", target_os = "macos")), - ignore = "POLLRDHUP isn't supported when using `poll` as a `kqueue` replacement on MacOS/FreeBSD." -)] fn unix_datagram_shutdown() { let (mut poll, mut events) = init_with_poll(); let path1 = temp_file("unix_datagram_shutdown1"); diff --git a/tests/unix_pipe.rs b/tests/unix_pipe.rs index 323934140..73c28419a 100644 --- a/tests/unix_pipe.rs +++ b/tests/unix_pipe.rs @@ -57,10 +57,6 @@ fn smoke() { any(target_os = "hurd", target_os = "nto"), ignore = "Writer fd close events do not trigger POLLHUP on nto and GNU/Hurd targets" )] -#[cfg_attr( - all(mio_unsupported_force_poll_poll, any(target_os = "macos")), - ignore = "Doesn't work on MacOS with `poll`." -)] fn event_when_sender_is_dropped() { let mut poll = Poll::new().unwrap(); let mut events = Events::with_capacity(8); @@ -103,10 +99,6 @@ fn event_when_sender_is_dropped() { any(target_os = "hurd", target_os = "nto"), ignore = "Writer fd close events do not trigger POLLHUP on nto and GNU/Hurd targets" )] -#[cfg_attr( - all(mio_unsupported_force_poll_poll, any(target_os = "macos")), - ignore = "Doesn't work on MacOS with `poll`." -)] fn event_when_receiver_is_dropped() { let mut poll = Poll::new().unwrap(); let mut events = Events::with_capacity(8); @@ -144,10 +136,6 @@ fn event_when_receiver_is_dropped() { any(target_os = "hurd", target_os = "nto"), ignore = "Writer fd close events do not trigger POLLHUP on nto and GNU/Hurd targets" )] -#[cfg_attr( - all(mio_unsupported_force_poll_poll, any(target_os = "macos")), - ignore = "Doesn't work on MacOS with `poll`." -)] fn from_child_process_io() { // `cat` simply echo everything that we write via standard in. let mut child = Command::new("cat") diff --git a/tests/unix_stream.rs b/tests/unix_stream.rs index 51ce37742..fda86bbc5 100644 --- a/tests/unix_stream.rs +++ b/tests/unix_stream.rs @@ -205,10 +205,6 @@ fn unix_stream_peer_addr() { #[cfg_attr(target_os = "hurd", ignore = "POLLRDHUP isn't supported on GNU/Hurd")] #[cfg_attr(target_os = "solaris", ignore = "POLLRDHUP isn't supported on Solaris")] #[cfg_attr(target_os = "nto", ignore = "POLLRDHUP isn't supported on NTO")] -#[cfg_attr( - all(mio_unsupported_force_poll_poll, any(target_os = "freebsd", target_os = "macos")), - ignore = "POLLRDHUP isn't supported when using `poll` as a `kqueue` replacement on FreeBSD/MacOS." -)] fn unix_stream_shutdown_read() { let (mut poll, mut events) = init_with_poll(); let (handle, remote_addr) = new_echo_listener(1, "unix_stream_shutdown_read"); @@ -298,19 +294,16 @@ fn unix_stream_shutdown_write() { stream.shutdown(Shutdown::Write).unwrap(); - #[cfg(all( - not(mio_unsupported_force_poll_poll), - any( - target_os = "dragonfly", - target_os = "freebsd", - target_os = "ios", - target_os = "macos", - target_os = "netbsd", - target_os = "openbsd", - target_os = "tvos", - target_os = "visionos", - target_os = "watchos" - ), + #[cfg(any( + target_os = "dragonfly", + target_os = "freebsd", + target_os = "ios", + target_os = "macos", + target_os = "netbsd", + target_os = "openbsd", + target_os = "tvos", + target_os = "visionos", + target_os = "watchos", ))] expect_events( &mut poll, @@ -335,10 +328,6 @@ fn unix_stream_shutdown_write() { target_os = "hurd", ignore = "getting pathname isn't supported on GNU/Hurd" )] -#[cfg_attr( - all(mio_unsupported_force_poll_poll, target_os = "macos"), - ignore = "Doesn't work on MacOS with `poll`." -)] fn unix_stream_shutdown_both() { let (mut poll, mut events) = init_with_poll(); let (handle, remote_addr) = new_echo_listener(1, "unix_stream_shutdown_both"); @@ -407,10 +396,6 @@ fn unix_stream_shutdown_both() { #[cfg_attr(target_os = "hurd", ignore = "POLLRDHUP isn't supported on GNU/Hurd")] #[cfg_attr(target_os = "solaris", ignore = "POLLRDHUP isn't supported on Solaris")] #[cfg_attr(target_os = "nto", ignore = "POLLRDHUP isn't supported on NTO")] -#[cfg_attr( - all(mio_unsupported_force_poll_poll, target_os = "freebsd"), - ignore = "POLLRDHUP isn't supported when using `poll` as a `kqueue` replacement on FreeBSD." -)] fn unix_stream_shutdown_listener_write() { let (mut poll, mut events) = init_with_poll(); let barrier = Arc::new(Barrier::new(2)); From 57a1312c7deab3682c40dfddedddb74904b52d16 Mon Sep 17 00:00:00 2001 From: Ivan Gankevich Date: Sat, 28 Dec 2024 17:22:54 +0100 Subject: [PATCH 17/27] Fix test condition. --- tests/process.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/process.rs b/tests/process.rs index eb3c52a4a..9c701e95d 100644 --- a/tests/process.rs +++ b/tests/process.rs @@ -22,6 +22,7 @@ target_os = "watchos", ), ), + windows, ), ))] From f82aae5e53e230cb5440f7feea70d055b1efbaca Mon Sep 17 00:00:00 2001 From: Ivan Gankevich Date: Mon, 30 Dec 2024 09:23:54 +0100 Subject: [PATCH 18/27] Fix docs. --- src/process.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/process.rs b/src/process.rs index 0fb96a0a2..4a2cdde31 100644 --- a/src/process.rs +++ b/src/process.rs @@ -5,7 +5,7 @@ use std::process::Child; /// Process allows polling OS processes for completion. /// -/// When the process exits the event with [`readable`](crate::event::Event::readable) readiness is generated. +/// When the process exits the event with _readable_ readiness is generated. /// /// # Notes /// From fccfe101de536b5563b9fa33904ca0b1b264f996 Mon Sep 17 00:00:00 2001 From: Ivan Gankevich Date: Mon, 30 Dec 2024 09:24:11 +0100 Subject: [PATCH 19/27] Fix rustfmt. --- src/macros.rs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/macros.rs b/src/macros.rs index d0311c0a4..c9601ca05 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -164,8 +164,10 @@ macro_rules! use_fd_traits { // TODO: once is fixed this // can use `std::os::fd` and be merged with the above. #[cfg(target_os = "hermit")] - use std::os::hermit::io::{AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd, OwnedFd, RawFd}; - } + use std::os::hermit::io::{ + AsFd, AsRawFd, BorrowedFd, FromRawFd, IntoRawFd, OwnedFd, RawFd, + }; + }; } macro_rules! trace { From 9fe8e3957863d2ebd7c68b8968487a27b834db7f Mon Sep 17 00:00:00 2001 From: Ivan Gankevich Date: Mon, 30 Dec 2024 09:25:20 +0100 Subject: [PATCH 20/27] Replace `PIDFD_NONBLOCK` with `O_NONBLOCK` to fix Android build. --- src/sys/unix/process/pidfd.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/sys/unix/process/pidfd.rs b/src/sys/unix/process/pidfd.rs index 09cc709f8..476d25f09 100644 --- a/src/sys/unix/process/pidfd.rs +++ b/src/sys/unix/process/pidfd.rs @@ -1,6 +1,6 @@ use crate::event::Source; use crate::{Interest, Registry, Token}; -use libc::{pid_t, SYS_pidfd_open, PIDFD_NONBLOCK}; +use libc::{pid_t, SYS_pidfd_open, O_NONBLOCK}; use std::fs::File; use std::io::Error; use std::process::Child; @@ -18,7 +18,8 @@ impl Process { } pub fn from_pid(pid: pid_t) -> Result { - let fd = syscall!(syscall(SYS_pidfd_open, pid, PIDFD_NONBLOCK))?; + // NB: `O_NONBLOCK` is the same as `PIDFD_NONBLOCK`. + let fd = syscall!(syscall(SYS_pidfd_open, pid, O_NONBLOCK))?; // SAFETY: `pidfd_open(2)` ensures the fd is valid. let fd = unsafe { File::from_raw_fd(fd as RawFd) }; Ok(Self { fd }) From 95af15937a0ccf6aa5f786ee3c7d885ad3fdf8dd Mon Sep 17 00:00:00 2001 From: Ivan Gankevich Date: Mon, 30 Dec 2024 09:42:23 +0100 Subject: [PATCH 21/27] Fix `kevent` flags for *bsd. --- src/sys/unix/selector/kqueue.rs | 38 ++++++++++++++++----------------- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/src/sys/unix/selector/kqueue.rs b/src/sys/unix/selector/kqueue.rs index 1169902fb..e7f505252 100644 --- a/src/sys/unix/selector/kqueue.rs +++ b/src/sys/unix/selector/kqueue.rs @@ -63,6 +63,21 @@ macro_rules! kevent { }; } +/// Same as `kevent` but infers `flags` from `interest`. +/// +/// Adds `EV_ADD` when `interest` is true, `EV_DELETE` otherwise. +macro_rules! kevent_update { + ($id: expr, $filter: expr, $interest: expr, $data: expr) => {{ + let flags = libc::EV_CLEAR | libc::EV_RECEIPT; + let add_delete = if $interest { + libc::EV_ADD + } else { + libc::EV_DELETE + }; + kevent!($id, $filter, flags | add_delete, $data) + }}; +} + #[derive(Debug)] pub struct Selector { #[cfg(debug_assertions)] @@ -154,12 +169,9 @@ impl Selector { } pub fn reregister(&self, fd: RawFd, token: Token, interests: Interest) -> io::Result<()> { - let write_flags = get_reregister_flags(interests.is_writable()); - let read_flags = get_reregister_flags(interests.is_readable()); - let mut changes: [libc::kevent; 2] = [ - kevent!(fd, libc::EVFILT_WRITE, write_flags, token.0), - kevent!(fd, libc::EVFILT_READ, read_flags, token.0), + kevent_update!(fd, libc::EVFILT_WRITE, interests.is_writable(), token.0), + kevent_update!(fd, libc::EVFILT_READ, interests.is_readable(), token.0), ]; // Since there is no way to check with which interests the fd was @@ -276,10 +288,10 @@ impl Selector { token: Token, interests: Interest, ) -> io::Result<()> { - let mut changes: [libc::kevent; 1] = [kevent!( + let mut changes: [libc::kevent; 1] = [kevent_update!( pid, libc::EVFILT_PROC, - get_reregister_flags(interests.is_readable()), + interests.is_readable(), token.0 )]; kevent_register(self.kq.as_raw_fd(), &mut changes, &[libc::ENOENT as i64]) @@ -339,18 +351,6 @@ fn check_errors(events: &[libc::kevent], ignored_errors: &[i64]) -> io::Result<( Ok(()) } -/// Get `reregister` flags for the specified `interest`. -/// -/// Returns standard flags plus `EV_ADD` when `interest` is true, `EV_DELETE` otherwise. -fn get_reregister_flags(interest: bool) -> u16 { - let flags = libc::EV_CLEAR | libc::EV_RECEIPT; - if interest { - flags | libc::EV_ADD - } else { - flags | libc::EV_DELETE - } -} - struct Changes { changes: [MaybeUninit; N], n_changes: usize, From 296ddedac5863a84f8ceaaff70f012142d00715e Mon Sep 17 00:00:00 2001 From: Ivan Gankevich Date: Mon, 30 Dec 2024 09:44:36 +0100 Subject: [PATCH 22/27] Enable `Process` for the same target OS as `kqueue`. --- src/macros.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/macros.rs b/src/macros.rs index c9601ca05..da92c1579 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -88,9 +88,12 @@ macro_rules! cfg_os_proc { all( not(mio_unsupported_force_poll_poll), any( + target_os = "dragonfly", target_os = "freebsd", target_os = "ios", target_os = "macos", + target_os = "netbsd", + target_os = "openbsd", target_os = "tvos", target_os = "visionos", target_os = "watchos", @@ -133,9 +136,12 @@ macro_rules! cfg_os_proc_kqueue { // `Process` needs `kqueue`. not(mio_unsupported_force_poll_poll), any( + target_os = "dragonfly", target_os = "freebsd", target_os = "ios", target_os = "macos", + target_os = "netbsd", + target_os = "openbsd", target_os = "tvos", target_os = "visionos", target_os = "watchos", From c3b5065abc7019b0739adf41e8aa58cca89497dc Mon Sep 17 00:00:00 2001 From: Ivan Gankevich Date: Mon, 30 Dec 2024 09:53:51 +0100 Subject: [PATCH 23/27] Disable `Process` on `illumos` and `redox`. --- src/macros.rs | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/src/macros.rs b/src/macros.rs index da92c1579..e48d4d4db 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -78,12 +78,7 @@ macro_rules! cfg_os_proc { feature = "os-proc", any( // pidfd (should be the same as in `cfg_os_proc_pidfd` macro) - any( - target_os = "android", - target_os = "illumos", - target_os = "linux", - target_os = "redox", - ), + any(target_os = "android", target_os = "linux"), // kqueue (should be the same as in `cfg_os_proc_kqueue` macro) all( not(mio_unsupported_force_poll_poll), @@ -99,7 +94,7 @@ macro_rules! cfg_os_proc { target_os = "watchos", ), ), - // windows + // windows (should be the same as in `cfg_os_proc_job_object` macro) windows, ), ), @@ -114,14 +109,7 @@ macro_rules! cfg_os_proc { macro_rules! cfg_os_proc_pidfd { ($($item:item)*) => { $( - #[cfg( - any( - target_os = "android", - target_os = "illumos", - target_os = "linux", - target_os = "redox", - ) - )] + #[cfg(any(target_os = "android", target_os = "linux"))] $item )* }; From 51af696b094effd27718e2f1c1a20baaafeb410a Mon Sep 17 00:00:00 2001 From: Ivan Gankevich Date: Mon, 30 Dec 2024 10:15:43 +0100 Subject: [PATCH 24/27] Fix sanitizers on Ubuntu 24.04. --- Makefile | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index a104bcda1..7887d409e 100644 --- a/Makefile +++ b/Makefile @@ -19,9 +19,11 @@ test_all: check_all_targets # compile correctly. test_sanitizer: @if [ -z $${SAN+x} ]; then echo "Required '\$$SAN' variable is not set" 1>&2; exit 1; fi - RUSTFLAGS="-Z sanitizer=$$SAN -Z sanitizer-memory-track-origins" \ - RUSTDOCFLAGS="-Z sanitizer=$$SAN -Z sanitizer-memory-track-origins" \ - cargo test -Z build-std --all-features --target $(RUSTUP_TARGET) + # We use `-Zexport-executable-symbols` here as a workaround for the following issue: + # https://github.com/rust-lang/rust/issues/111073 + RUSTFLAGS="-Z sanitizer=$$SAN -Z sanitizer-memory-track-origins -Zexport-executable-symbols" \ + RUSTDOCFLAGS="-Z sanitizer=$$SAN -Z sanitizer-memory-track-origins -Zexport-executable-symbols" \ + cargo test -Z build-std --all-features --target $(RUSTUP_TARGET) -- --nocapture # Check all targets using all features. check_all_targets: $(TARGETS) From dd27a47d94d88c5b4deb4fe9ebfe2ae0cf8d5fd2 Mon Sep 17 00:00:00 2001 From: Ivan Gankevich Date: Mon, 30 Dec 2024 10:38:03 +0100 Subject: [PATCH 25/27] Fix `cfg` conditionals in `tests/process.rs`. --- tests/process.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/tests/process.rs b/tests/process.rs index 9c701e95d..4ea091fdc 100644 --- a/tests/process.rs +++ b/tests/process.rs @@ -3,25 +3,24 @@ feature = "net", feature = "os-proc", any( - // pidfd - any( - target_os = "android", - target_os = "illumos", - target_os = "linux", - target_os = "redox", - ), - // kqueue + // pidfd (should be the same as in `cfg_os_proc_pidfd` macro) + any(target_os = "android", target_os = "linux"), + // kqueue (should be the same as in `cfg_os_proc_kqueue` macro) all( not(mio_unsupported_force_poll_poll), any( + target_os = "dragonfly", target_os = "freebsd", target_os = "ios", target_os = "macos", + target_os = "netbsd", + target_os = "openbsd", target_os = "tvos", target_os = "visionos", target_os = "watchos", ), ), + // windows (should be the same as in `cfg_os_proc_job_object` macro) windows, ), ))] From e127a66945499c6b399843b2ffb3d1bab3fd37ed Mon Sep 17 00:00:00 2001 From: Ivan Gankevich Date: Mon, 30 Dec 2024 17:32:25 +0100 Subject: [PATCH 26/27] Continue to `poll` after interrupt in tests. --- tests/util/mod.rs | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/util/mod.rs b/tests/util/mod.rs index fee97e320..43b48c8c7 100644 --- a/tests/util/mod.rs +++ b/tests/util/mod.rs @@ -147,7 +147,14 @@ pub fn expect_events(poll: &mut Poll, events: &mut Events, mut expected: Vec {} + Err(ref e) if e.kind() == io::ErrorKind::Interrupted => { + warn!("poll interrupted"); + continue; + } + Err(e) => panic!("failed to poll: {}", e), + } if t.elapsed() >= TIMEOUT && events.is_empty() { warn!("poll timed out"); From 2130447f5afe1f1c34575214cc094809c8ef6b73 Mon Sep 17 00:00:00 2001 From: Ivan Gankevich Date: Mon, 30 Dec 2024 18:15:28 +0100 Subject: [PATCH 27/27] Disable leak sanitizer in panicking tests. --- Makefile | 4 ++-- tests/waker.rs | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/Makefile b/Makefile index 7887d409e..f8776dd78 100644 --- a/Makefile +++ b/Makefile @@ -21,8 +21,8 @@ test_sanitizer: @if [ -z $${SAN+x} ]; then echo "Required '\$$SAN' variable is not set" 1>&2; exit 1; fi # We use `-Zexport-executable-symbols` here as a workaround for the following issue: # https://github.com/rust-lang/rust/issues/111073 - RUSTFLAGS="-Z sanitizer=$$SAN -Z sanitizer-memory-track-origins -Zexport-executable-symbols" \ - RUSTDOCFLAGS="-Z sanitizer=$$SAN -Z sanitizer-memory-track-origins -Zexport-executable-symbols" \ + RUSTFLAGS="-Z sanitizer=$$SAN -Z sanitizer-memory-track-origins -Zexport-executable-symbols --cfg mio_sanitize_$$SAN" \ + RUSTDOCFLAGS="-Z sanitizer=$$SAN -Z sanitizer-memory-track-origins -Zexport-executable-symbols --cfg mio_sanitize_$$SAN" \ cargo test -Z build-std --all-features --target $(RUSTUP_TARGET) -- --nocapture # Check all targets using all features. diff --git a/tests/waker.rs b/tests/waker.rs index 0893b0fd3..1c457aa27 100644 --- a/tests/waker.rs +++ b/tests/waker.rs @@ -112,6 +112,7 @@ fn waker_multiple_wakeups_different_thread() { not(debug_assertions), ignore = "only works with debug_assertions enabled" )] +#[cfg_attr(not(mio_sanitize_leak), ignore = "Doesn't work under `leak` sanitizer")] #[should_panic = "Only a single `Waker` can be active per `Poll` instance"] fn using_multiple_wakers_panics() { init(); @@ -133,6 +134,7 @@ fn using_multiple_wakers_panics() { not(debug_assertions), ignore = "only works with debug_assertions enabled" )] +#[cfg_attr(not(mio_sanitize_leak), ignore = "Doesn't work under `leak` sanitizer")] #[should_panic = "Only a single `Waker` can be active per `Poll` instance"] fn using_multiple_wakers_panics_different_cloned_registries() { init();