Skip to content

Commit

Permalink
refactor: remove child ownership from Exit filter (#207)
Browse files Browse the repository at this point in the history
Pid is used instead
  • Loading branch information
rogercoll authored Oct 18, 2024
1 parent 7d29a93 commit 9713083
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 12 deletions.
19 changes: 16 additions & 3 deletions src/os/kqueue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use crate::Async;

use std::future::Future;
use std::io::{Error, Result};
use std::num::NonZeroI32;
use std::os::unix::io::{AsFd, AsRawFd, BorrowedFd, OwnedFd, RawFd};
use std::pin::Pin;
use std::process::Child;
Expand Down Expand Up @@ -238,18 +239,30 @@ impl Queueable for Signal {}
/// When registered into [`Async`](crate::Async) via [`with_filter`](AsyncKqueueExt::with_filter),
/// it will return a [`readable`](crate::Async::readable) event when the child process exits.
#[derive(Debug)]
pub struct Exit(Option<Child>);
pub struct Exit(NonZeroI32);

impl Exit {
/// Create a new `Exit` object.
pub fn new(child: Child) -> Self {
Self(Some(child))
Self(
NonZeroI32::new(child.id().try_into().expect("unable to parse pid"))
.expect("cannot register pid with zero value"),
)
}

/// Create a new `Exit` object from a PID.
///
/// # Safety
///
/// The PID must be tied to an actual child process.
pub unsafe fn from_pid(pid: NonZeroI32) -> Self {
Self(pid)
}
}

impl QueueableSealed for Exit {
fn registration(&mut self) -> Registration {
Registration::Process(self.0.take().expect("Cannot reregister child"))
Registration::Process(self.0)
}
}
impl Queueable for Exit {}
Expand Down
18 changes: 9 additions & 9 deletions src/reactor/kqueue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ use polling::{Event, PollMode, Poller};

use std::fmt;
use std::io::Result;
use std::num::NonZeroI32;
use std::os::unix::io::{AsRawFd, BorrowedFd, RawFd};
use std::process::Child;

/// The raw registration into the reactor.
///
Expand All @@ -27,8 +27,8 @@ pub enum Registration {
/// Raw signal number for signal delivery.
Signal(Signal),

/// Process for process termination.
Process(Child),
/// Pid for process termination.
Process(NonZeroI32),
}

impl fmt::Debug for Registration {
Expand Down Expand Up @@ -62,8 +62,8 @@ impl Registration {
Self::Signal(signal) => {
poller.add_filter(PollSignal(signal.0), token, PollMode::Oneshot)
}
Self::Process(process) => poller.add_filter(
unsafe { Process::new(process, ProcessOps::Exit) },
Self::Process(pid) => poller.add_filter(
unsafe { Process::from_pid(*pid, ProcessOps::Exit) },
token,
PollMode::Oneshot,
),
Expand All @@ -82,8 +82,8 @@ impl Registration {
Self::Signal(signal) => {
poller.modify_filter(PollSignal(signal.0), interest.key, PollMode::Oneshot)
}
Self::Process(process) => poller.modify_filter(
unsafe { Process::new(process, ProcessOps::Exit) },
Self::Process(pid) => poller.modify_filter(
unsafe { Process::from_pid(*pid, ProcessOps::Exit) },
interest.key,
PollMode::Oneshot,
),
Expand All @@ -100,8 +100,8 @@ impl Registration {
poller.delete(fd)
}
Self::Signal(signal) => poller.delete_filter(PollSignal(signal.0)),
Self::Process(process) => {
poller.delete_filter(unsafe { Process::new(process, ProcessOps::Exit) })
Self::Process(pid) => {
poller.delete_filter(unsafe { Process::from_pid(*pid, ProcessOps::Exit) })
}
}
}
Expand Down

0 comments on commit 9713083

Please sign in to comment.