From 536ac262a126b6df30cb5e0d22eddda367e1276d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fe=CC=81lix=20Saparelli?= Date: Sat, 28 Jun 2025 06:57:41 +1200 Subject: [PATCH 01/18] bump msrv to 86 for upcasting --- .github/workflows/test.yml | 2 +- Cargo.toml | 2 +- README.md | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index dcc3e54..bc26987 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -16,7 +16,7 @@ jobs: - windows toolchain: - stable - - 1.77.0 + - 1.86.0 features: - tokio1 - std diff --git a/Cargo.toml b/Cargo.toml index 94e543e..5ca58f6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,7 +14,7 @@ readme = "README.md" edition = "2021" exclude = ["/bin", "/.github"] -rust-version = "1.77.0" +rust-version = "1.86.0" [dependencies] futures = { version = "0.3.30", optional = true } diff --git a/README.md b/README.md index 1e540dd..cdeaa58 100644 --- a/README.md +++ b/README.md @@ -7,7 +7,7 @@ - **[API documentation][docs]**. - [Dual-licensed][copyright] with Apache 2.0 and MIT. - Successor to [command-group](https://github.com/watchexec/command-group). -- Minimum Supported Rust Version: 1.77.0. +- Minimum Supported Rust Version: 1.86.0. - Only the latest stable rustc version is supported. - MSRV increases will not incur major version bumps. From 7e1211080b4ff64b0187d898206f858a2ce7a750 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fe=CC=81lix=20Saparelli?= Date: Sat, 28 Jun 2025 06:59:19 +1200 Subject: [PATCH 02/18] remove downcasting feature --- Cargo.toml | 3 --- src/generic_wrap.rs | 16 ---------------- src/tokio/core.rs | 2 -- 3 files changed, 21 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 5ca58f6..920483d 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -41,9 +41,6 @@ tokio = { version = "1.38.2", features = ["io-util", "macros", "process", "rt", [features] default = ["creation-flags", "job-object", "kill-on-drop", "process-group", "process-session", "tracing"] -## Enable Any trait bound on the ChildWrapper traits -downcasting = [] - ## Enable internal tracing logs tracing = ["dep:tracing"] diff --git a/src/generic_wrap.rs b/src/generic_wrap.rs index 6583c32..ce9c393 100644 --- a/src/generic_wrap.rs +++ b/src/generic_wrap.rs @@ -230,20 +230,4 @@ macro_rules! Wrap { }; } -macro_rules! MaybeAnyTrait { - ( - $(#[$($attrss:tt)*])* - $v:vis trait $name:ident $body:tt - ) => { - #[cfg(feature = "downcasting")] - $(#[$($attrss)*])* - $v trait $name: std::fmt::Debug + Send + Sync + std::any::Any $body - - #[cfg(not(feature = "downcasting"))] - $(#[$($attrss)*])* - $v trait $name: std::fmt::Debug + Send + Sync $body - } -} - -pub(crate) use MaybeAnyTrait; pub(crate) use Wrap; diff --git a/src/tokio/core.rs b/src/tokio/core.rs index 4806693..94b0ccb 100644 --- a/src/tokio/core.rs +++ b/src/tokio/core.rs @@ -24,7 +24,6 @@ crate::generic_wrap::Wrap!( |child| child ); -crate::generic_wrap::MaybeAnyTrait! { /// Wrapper for `tokio::process::Child`. /// /// This trait exposes most of the functionality of the underlying [`Child`]. It is implemented for @@ -213,7 +212,6 @@ pub trait TokioChildWrapper { } } } -} impl TokioChildWrapper for Child { fn inner(&self) -> &Child { From 4dae8abc00c186d81e9ed5eded020ecea474358c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fe=CC=81lix=20Saparelli?= Date: Sat, 28 Jun 2025 07:05:03 +1200 Subject: [PATCH 03/18] fix unused controlflow warns --- src/tokio/job_object.rs | 4 ++-- src/tokio/process_group.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/tokio/job_object.rs b/src/tokio/job_object.rs index ca7c804..2b56de3 100644 --- a/src/tokio/job_object.rs +++ b/src/tokio/job_object.rs @@ -161,14 +161,14 @@ impl TokioChildWrapper for JobObjectChild { let JobPort { completion_port, .. } = self.job_port; - spawn_blocking(move || wait_on_job(completion_port, None)).await??; + let _ = spawn_blocking(move || wait_on_job(completion_port, None)).await??; Ok(status) }) } #[cfg_attr(feature = "tracing", instrument(level = "debug", skip(self)))] fn try_wait(&mut self) -> Result> { - wait_on_job(self.job_port.completion_port, Some(Duration::ZERO))?; + let _ = wait_on_job(self.job_port.completion_port, Some(Duration::ZERO))?; self.inner.try_wait() } } diff --git a/src/tokio/process_group.rs b/src/tokio/process_group.rs index f01a9e2..d162f7d 100644 --- a/src/tokio/process_group.rs +++ b/src/tokio/process_group.rs @@ -201,7 +201,7 @@ impl TokioChildWrapper for ProcessGroupChild { // ...finally, if there are some that are still alive, // block in the background to reap them fully. - spawn_blocking(move || Self::wait_imp(pgid, WaitPidFlag::empty())).await??; + let _ = spawn_blocking(move || Self::wait_imp(pgid, WaitPidFlag::empty())).await??; Ok(status) }) } From 49c47ca4e59b30fe0c5532737390f5c517ebde92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fe=CC=81lix=20Saparelli?= Date: Sat, 28 Jun 2025 07:13:08 +1200 Subject: [PATCH 04/18] change child wrapper inner()s to return one layer down --- src/tokio/core.rs | 114 ++++++++++++++++++++++++++++--------- src/tokio/job_object.rs | 18 +++--- src/tokio/process_group.rs | 18 +++--- 3 files changed, 103 insertions(+), 47 deletions(-) diff --git a/src/tokio/core.rs b/src/tokio/core.rs index 94b0ccb..44c9270 100644 --- a/src/tokio/core.rs +++ b/src/tokio/core.rs @@ -1,6 +1,8 @@ use std::{ + any::Any, future::Future, io::Result, + pin::Pin, process::{ExitStatus, Output}, }; @@ -46,32 +48,32 @@ crate::generic_wrap::Wrap!( /// pub struct YourChildWrapper(Child); /// /// impl TokioChildWrapper for YourChildWrapper { -/// fn inner(&self) -> &Child { +/// fn inner(&self) -> &dyn TokioChildWrapper { /// &self.0 /// } /// -/// fn inner_mut(&mut self) -> &mut Child { +/// fn inner_mut(&mut self) -> &mut dyn TokioChildWrapper { /// &mut self.0 /// } /// -/// fn into_inner(self: Box) -> Child { -/// (*self).0 +/// fn into_inner(self: Box) -> Box { +/// Box::new((*self).0) /// } /// } /// ``` -pub trait TokioChildWrapper { - /// Obtain a reference to the underlying `Child`. - fn inner(&self) -> &Child; +pub trait TokioChildWrapper: Any + std::fmt::Debug + Send { + /// Obtain a reference to the wrapped child. + fn inner(&self) -> &dyn TokioChildWrapper; - /// Obtain a mutable reference to the underlying `Child`. - fn inner_mut(&mut self) -> &mut Child; + /// Obtain a mutable reference to the wrapped child. + fn inner_mut(&mut self) -> &mut dyn TokioChildWrapper; - /// Consume the wrapper and return the underlying `Child`. + /// Consume the current wrapper and return the wrapped child. /// - /// Note that this may disrupt whatever the wrappers were doing. However, wrappers must ensure - /// that the `Child` is in a consistent state when this is called or they are dropped, so that - /// this is always safe. - fn into_inner(self: Box) -> Child; + /// Note that this may disrupt whatever the current wrapper was doing. However, wrappers must + /// ensure that the wrapped child is in a consistent state when this is called or they are + /// dropped, so that this is always safe. + fn into_inner(self: Box) -> Box; /// Obtain a clone if possible. /// @@ -83,23 +85,23 @@ pub trait TokioChildWrapper { /// Obtain the `Child`'s stdin. /// - /// By default this is a passthrough to the underlying `Child`. + /// By default this is a passthrough to the wrapped child. fn stdin(&mut self) -> &mut Option { - &mut self.inner_mut().stdin + self.inner_mut().stdin() } /// Obtain the `Child`'s stdout. /// - /// By default this is a passthrough to the underlying `Child`. + /// By default this is a passthrough to the wrapped child. fn stdout(&mut self) -> &mut Option { - &mut self.inner_mut().stdout + self.inner_mut().stdout() } /// Obtain the `Child`'s stderr. /// - /// By default this is a passthrough to the underlying `Child`. + /// By default this is a passthrough to the wrapped child. fn stderr(&mut self) -> &mut Option { - &mut self.inner_mut().stderr + self.inner_mut().stderr() } /// Obtain the `Child`'s process ID. @@ -120,7 +122,7 @@ pub trait TokioChildWrapper { fn kill(&mut self) -> Box> + Send + '_> { Box::new(async { self.start_kill()?; - Box::into_pin(self.wait()).await?; + self.wait().await?; Ok(()) }) } @@ -150,8 +152,8 @@ pub trait TokioChildWrapper { /// has exited will always return the same result. /// /// By default this is a passthrough to the underlying `Child`. - fn wait(&mut self) -> Box> + Send + '_> { - Box::new(self.inner_mut().wait()) + fn wait(&mut self) -> Pin> + Send + '_>> { + Box::pin(self.inner_mut().wait()) } /// Wait for the `Child` to exit and return its exit status and outputs. @@ -180,7 +182,7 @@ pub trait TokioChildWrapper { let stderr_fut = read_to_end(&mut stderr_pipe); let (status, stdout, stderr) = - try_join3(Box::into_pin(self.wait()), stdout_fut, stderr_fut).await?; + try_join3(self.wait(), stdout_fut, stderr_fut).await?; // Drop happens after `try_join` due to drop(stdout_pipe); @@ -214,13 +216,69 @@ pub trait TokioChildWrapper { } impl TokioChildWrapper for Child { - fn inner(&self) -> &Child { + fn inner(&self) -> &dyn TokioChildWrapper { self } - fn inner_mut(&mut self) -> &mut Child { + fn inner_mut(&mut self) -> &mut dyn TokioChildWrapper { self } - fn into_inner(self: Box) -> Child { - *self + fn into_inner(self: Box) -> Box { + Box::new(*self) + } + fn stdin(&mut self) -> &mut Option { + &mut self.stdin + } + fn stdout(&mut self) -> &mut Option { + &mut self.stdout + } + fn stderr(&mut self) -> &mut Option { + &mut self.stderr + } +} + +impl dyn TokioChildWrapper { + fn downcast_ref(&self) -> Option<&T> { + (self as &dyn Any).downcast_ref() + } + + fn is_raw_child(&self) -> bool { + self.downcast_ref::().is_some() + } + + /// Obtain a reference to the underlying [`Child`]. + pub fn inner_child(&self) -> &Child { + let mut inner = self; + while !inner.is_raw_child() { + inner = inner.inner(); + } + + // UNWRAP: we've just checked that it's Some with is_raw_child() + inner.downcast_ref().unwrap() + } + + /// Obtain a mutable reference to the underlying [`Child`]. + /// + /// Modifying the raw child may be unsound depending on the layering of wrappers. + pub unsafe fn inner_child_mut(&mut self) -> &mut Child { + let mut inner = self; + while !inner.is_raw_child() { + inner = inner.inner_mut(); + } + + // UNWRAP: we've just checked that with is_raw_child() + (inner as &mut dyn Any).downcast_mut().unwrap() + } + + /// Obtain the underlying [`Child`]. + /// + /// Unwrapping everything may be unsound depending on the state of the wrappers. + pub unsafe fn into_inner_child(self: Box) -> Child { + let mut inner = self; + while !inner.is_raw_child() { + inner = inner.into_inner(); + } + + // UNWRAP: we've just checked that with is_raw_child() + *(inner as Box).downcast().unwrap() } } diff --git a/src/tokio/job_object.rs b/src/tokio/job_object.rs index 2b56de3..9e1322f 100644 --- a/src/tokio/job_object.rs +++ b/src/tokio/job_object.rs @@ -1,7 +1,7 @@ -use std::{future::Future, io::Result, process::ExitStatus, time::Duration}; +use std::{future::Future, io::Result, pin::Pin, process::ExitStatus, time::Duration}; use tokio::{ - process::{Child, Command}, + process::Command, task::spawn_blocking, }; #[cfg(feature = "tracing")] @@ -78,7 +78,7 @@ impl TokioCommandWrapper for JobObject { let handle = HANDLE( inner - .inner() + .inner_child() .raw_handle() .expect("child has exited but it has not even started") as _, ); @@ -114,13 +114,13 @@ impl JobObjectChild { } impl TokioChildWrapper for JobObjectChild { - fn inner(&self) -> &Child { + fn inner(&self) -> &dyn TokioChildWrapper { self.inner.inner() } - fn inner_mut(&mut self) -> &mut Child { + fn inner_mut(&mut self) -> &mut dyn TokioChildWrapper { self.inner.inner_mut() } - fn into_inner(self: Box) -> Child { + fn into_inner(self: Box) -> Box { // manually drop the completion port let its = std::mem::ManuallyDrop::new(self.job_port); unsafe { CloseHandle(its.completion_port.0) }.ok(); @@ -136,8 +136,8 @@ impl TokioChildWrapper for JobObjectChild { } #[cfg_attr(feature = "tracing", instrument(level = "debug", skip(self)))] - fn wait(&mut self) -> Box> + Send + '_> { - Box::new(async { + fn wait(&mut self) -> Pin> + Send + '_>> { + Box::pin(async { if let ChildExitStatus::Exited(status) = &self.exit_status { return Ok(*status); } @@ -146,7 +146,7 @@ impl TokioChildWrapper for JobObjectChild { // always wait for parent to exit first, as by the time it does, // it's likely that all its children have already exited. - let status = Box::into_pin(self.inner.wait()).await?; + let status = self.inner.wait().await?; self.exit_status = ChildExitStatus::Exited(status); // nevertheless, now try reaping all children a few times... diff --git a/src/tokio/process_group.rs b/src/tokio/process_group.rs index d162f7d..6b45255 100644 --- a/src/tokio/process_group.rs +++ b/src/tokio/process_group.rs @@ -3,6 +3,7 @@ use std::{ io::{Error, Result}, ops::ControlFlow, os::unix::process::ExitStatusExt, + pin::Pin, process::ExitStatus, }; @@ -15,10 +16,7 @@ use nix::{ }, unistd::Pid, }; -use tokio::{ - process::{Child, Command}, - task::spawn_blocking, -}; +use tokio::{process::Command, task::spawn_blocking}; #[cfg(feature = "tracing")] use tracing::instrument; @@ -162,13 +160,13 @@ impl ProcessGroupChild { } impl TokioChildWrapper for ProcessGroupChild { - fn inner(&self) -> &Child { + fn inner(&self) -> &dyn TokioChildWrapper { self.inner.inner() } - fn inner_mut(&mut self) -> &mut Child { + fn inner_mut(&mut self) -> &mut dyn TokioChildWrapper { self.inner.inner_mut() } - fn into_inner(self: Box) -> Child { + fn into_inner(self: Box) -> Box { self.inner.into_inner() } @@ -178,8 +176,8 @@ impl TokioChildWrapper for ProcessGroupChild { } #[cfg_attr(feature = "tracing", instrument(level = "debug", skip(self)))] - fn wait(&mut self) -> Box> + Send + '_> { - Box::new(async { + fn wait(&mut self) -> Pin> + Send + '_>> { + Box::pin(async { if let ChildExitStatus::Exited(status) = &self.exit_status { return Ok(*status); } @@ -189,7 +187,7 @@ impl TokioChildWrapper for ProcessGroupChild { // always wait for parent to exit first, as by the time it does, // it's likely that all its children have already been reaped. - let status = Box::into_pin(self.inner.wait()).await?; + let status = self.inner.wait().await?; self.exit_status = ChildExitStatus::Exited(status); // nevertheless, now try reaping all children a few times... From 1a6f94c077dbf3a8db7dbe15e384f23fb77becab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fe=CC=81lix=20Saparelli?= Date: Sat, 28 Jun 2025 07:32:51 +1200 Subject: [PATCH 05/18] impl for std --- src/std/core.rs | 114 +++++++++++++++++++++++++++++---------- src/std/job_object.rs | 10 ++-- src/std/process_group.rs | 10 ++-- 3 files changed, 96 insertions(+), 38 deletions(-) diff --git a/src/std/core.rs b/src/std/core.rs index e5ec4e6..d6e63c0 100644 --- a/src/std/core.rs +++ b/src/std/core.rs @@ -1,4 +1,5 @@ use std::{ + any::Any, io::{Read, Result}, process::{Child, ChildStderr, ChildStdin, ChildStdout, Command, ExitStatus, Output}, }; @@ -18,7 +19,6 @@ crate::generic_wrap::Wrap!( StdChild // |child| StdChild(child) ); -crate::generic_wrap::MaybeAnyTrait! { /// Wrapper for `std::process::Child`. /// /// This trait exposes most of the functionality of the underlying [`Child`]. It is implemented for @@ -42,32 +42,32 @@ crate::generic_wrap::MaybeAnyTrait! { /// pub struct YourChildWrapper(Child); /// /// impl StdChildWrapper for YourChildWrapper { -/// fn inner(&self) -> &Child { +/// fn inner(&self) -> &dyn StdChildWrapper { /// &self.0 /// } /// -/// fn inner_mut(&mut self) -> &mut Child { +/// fn inner_mut(&mut self) -> &mut dyn StdChildWrapper { /// &mut self.0 /// } /// -/// fn into_inner(self: Box) -> Child { -/// (*self).0 +/// fn into_inner(self: Box) -> Box { +/// Box::new((*self).0) /// } /// } /// ``` -pub trait StdChildWrapper { - /// Obtain a reference to the underlying `Child`. - fn inner(&self) -> &Child; +pub trait StdChildWrapper: Any + std::fmt::Debug + Send { + /// Obtain a reference to the wrapped child. + fn inner(&self) -> &dyn StdChildWrapper; - /// Obtain a mutable reference to the underlying `Child`. - fn inner_mut(&mut self) -> &mut Child; + /// Obtain a mutable reference to the wrapped child. + fn inner_mut(&mut self) -> &mut dyn StdChildWrapper; - /// Consume the wrapper and return the underlying `Child`. + /// Consume the current wrapper and return the wrapped child. /// - /// Note that this may disrupt whatever the wrappers were doing. However, wrappers must ensure - /// that the `Child` is in a consistent state when this is called or they are dropped, so that - /// this is always safe. - fn into_inner(self: Box) -> Child; + /// Note that this may disrupt whatever the current wrapper was doing. However, wrappers must + /// ensure that the wrapped child is in a consistent state when this is called or they are + /// dropped, so that this is always safe. + fn into_inner(self: Box) -> Box; /// Obtain a clone if possible. /// @@ -79,23 +79,23 @@ pub trait StdChildWrapper { /// Obtain the `Child`'s stdin. /// - /// By default this is a passthrough to the underlying `Child`. + /// By default this is a passthrough to the wrapped child. fn stdin(&mut self) -> &mut Option { - &mut self.inner_mut().stdin + self.inner_mut().stdin() } /// Obtain the `Child`'s stdout. /// - /// By default this is a passthrough to the underlying `Child`. + /// By default this is a passthrough to the wrapped child. fn stdout(&mut self) -> &mut Option { - &mut self.inner_mut().stdout + self.inner_mut().stdout() } /// Obtain the `Child`'s stderr. /// - /// By default this is a passthrough to the underlying `Child`. + /// By default this is a passthrough to the wrapped child. fn stderr(&mut self) -> &mut Option { - &mut self.inner_mut().stderr + self.inner_mut().stderr() } /// Obtain the `Child`'s process ID. @@ -212,7 +212,6 @@ pub trait StdChildWrapper { .map_err(std::io::Error::from) } } -} /// A thin wrapper around [`Child`]. /// @@ -223,14 +222,73 @@ pub trait StdChildWrapper { pub struct StdChild(pub Child); impl StdChildWrapper for StdChild { - fn inner(&self) -> &Child { - &self.0 + fn inner(&self) -> &dyn StdChildWrapper { + self } - fn inner_mut(&mut self) -> &mut Child { - &mut self.0 + fn inner_mut(&mut self) -> &mut dyn StdChildWrapper { + self } - fn into_inner(self: Box) -> Child { - (*self).0 + fn into_inner(self: Box) -> Box { + self + } + fn stdin(&mut self) -> &mut Option { + &mut self.0.stdin + } + fn stdout(&mut self) -> &mut Option { + &mut self.0.stdout + } + fn stderr(&mut self) -> &mut Option { + &mut self.0.stderr + } +} + +impl dyn StdChildWrapper { + fn downcast_ref(&self) -> Option<&T> { + (self as &dyn Any).downcast_ref() + } + + fn is_raw_child(&self) -> bool { + self.downcast_ref::().is_some() + } + + /// Obtain a reference to the underlying [`Child`]. + pub fn inner_child(&self) -> &Child { + let mut inner = self; + while !inner.is_raw_child() { + inner = inner.inner(); + } + + // UNWRAP: we've just checked that it's Some with is_raw_child() + &inner.downcast_ref::().unwrap().0 + } + + /// Obtain a mutable reference to the underlying [`Child`]. + /// + /// Modifying the raw child may be unsound depending on the layering of wrappers. + pub unsafe fn inner_child_mut(&mut self) -> &mut Child { + let mut inner = self; + while !inner.is_raw_child() { + inner = inner.inner_mut(); + } + + // UNWRAP: we've just checked that with is_raw_child() + &mut (inner as &mut dyn Any) + .downcast_mut::() + .unwrap() + .0 + } + + /// Obtain the underlying [`Child`]. + /// + /// Unwrapping everything may be unsound depending on the state of the wrappers. + pub unsafe fn into_inner_child(self: Box) -> Child { + let mut inner = self; + while !inner.is_raw_child() { + inner = inner.into_inner(); + } + + // UNWRAP: we've just checked that with is_raw_child() + (inner as Box).downcast::().unwrap().0 } } diff --git a/src/std/job_object.rs b/src/std/job_object.rs index 574de65..8c7bc47 100644 --- a/src/std/job_object.rs +++ b/src/std/job_object.rs @@ -1,7 +1,7 @@ use std::{ io::Result, os::windows::{io::AsRawHandle, process::CommandExt}, - process::{Child, Command, ExitStatus}, + process::{Command, ExitStatus}, time::Duration, }; @@ -66,7 +66,7 @@ impl StdCommandWrapper for JobObject { #[cfg(feature = "tracing")] debug!(?create_suspended, "options from other wrappers"); - let handle = HANDLE(inner.inner().as_raw_handle() as _); + let handle = HANDLE(inner.inner_child().as_raw_handle() as _); let job_port = make_job_object(handle, false)?; @@ -99,13 +99,13 @@ impl JobObjectChild { } impl StdChildWrapper for JobObjectChild { - fn inner(&self) -> &Child { + fn inner(&self) -> &dyn StdChildWrapper { self.inner.inner() } - fn inner_mut(&mut self) -> &mut Child { + fn inner_mut(&mut self) -> &mut dyn StdChildWrapper { self.inner.inner_mut() } - fn into_inner(self: Box) -> Child { + fn into_inner(self: Box) -> Box { // manually drop the completion port let its = std::mem::ManuallyDrop::new(self.job_port); unsafe { CloseHandle(its.completion_port.0) }.ok(); diff --git a/src/std/process_group.rs b/src/std/process_group.rs index ff94518..d34356d 100644 --- a/src/std/process_group.rs +++ b/src/std/process_group.rs @@ -2,7 +2,7 @@ use std::{ io::{Error, Result}, ops::ControlFlow, os::unix::process::{CommandExt, ExitStatusExt}, - process::{Child, Command, ExitStatus}, + process::{Command, ExitStatus}, }; use nix::{ @@ -150,13 +150,13 @@ impl ProcessGroupChild { } impl StdChildWrapper for ProcessGroupChild { - fn inner(&self) -> &Child { + fn inner(&self) -> &dyn StdChildWrapper { self.inner.inner() } - fn inner_mut(&mut self) -> &mut Child { + fn inner_mut(&mut self) -> &mut dyn StdChildWrapper { self.inner.inner_mut() } - fn into_inner(self: Box) -> Child { + fn into_inner(self: Box) -> Box { self.inner.into_inner() } @@ -177,7 +177,7 @@ impl StdChildWrapper for ProcessGroupChild { self.exit_status = ChildExitStatus::Exited(status); // nevertheless, now wait and make sure we reap all children. - Self::wait_imp(self.pgid, WaitPidFlag::empty())?; + let _ = Self::wait_imp(self.pgid, WaitPidFlag::empty())?; Ok(status) } From 31a40b7d36b55acb64b924278df0e28e64134802 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fe=CC=81lix=20Saparelli?= Date: Sat, 28 Jun 2025 07:34:13 +1200 Subject: [PATCH 06/18] fmt --- src/tokio/core.rs | 7 +++---- src/tokio/job_object.rs | 5 +---- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/src/tokio/core.rs b/src/tokio/core.rs index 44c9270..49b3b86 100644 --- a/src/tokio/core.rs +++ b/src/tokio/core.rs @@ -181,8 +181,7 @@ pub trait TokioChildWrapper: Any + std::fmt::Debug + Send { let stdout_fut = read_to_end(&mut stdout_pipe); let stderr_fut = read_to_end(&mut stderr_pipe); - let (status, stdout, stderr) = - try_join3(self.wait(), stdout_fut, stderr_fut).await?; + let (status, stdout, stderr) = try_join3(self.wait(), stdout_fut, stderr_fut).await?; // Drop happens after `try_join` due to drop(stdout_pipe); @@ -238,8 +237,8 @@ impl TokioChildWrapper for Child { impl dyn TokioChildWrapper { fn downcast_ref(&self) -> Option<&T> { - (self as &dyn Any).downcast_ref() - } + (self as &dyn Any).downcast_ref() + } fn is_raw_child(&self) -> bool { self.downcast_ref::().is_some() diff --git a/src/tokio/job_object.rs b/src/tokio/job_object.rs index 9e1322f..a5cf64a 100644 --- a/src/tokio/job_object.rs +++ b/src/tokio/job_object.rs @@ -1,9 +1,6 @@ use std::{future::Future, io::Result, pin::Pin, process::ExitStatus, time::Duration}; -use tokio::{ - process::Command, - task::spawn_blocking, -}; +use tokio::{process::Command, task::spawn_blocking}; #[cfg(feature = "tracing")] use tracing::{debug, instrument}; use windows::Win32::{ From ee807252f1b99fa51aea108ad93c2553f0f78da8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fe=CC=81lix=20Saparelli?= Date: Sat, 28 Jun 2025 07:36:09 +1200 Subject: [PATCH 07/18] fix test check --- tests/std_unix/into_inner_write_stdin.rs | 40 ++++++++++++++---------- 1 file changed, 23 insertions(+), 17 deletions(-) diff --git a/tests/std_unix/into_inner_write_stdin.rs b/tests/std_unix/into_inner_write_stdin.rs index 79a430b..d133e12 100644 --- a/tests/std_unix/into_inner_write_stdin.rs +++ b/tests/std_unix/into_inner_write_stdin.rs @@ -2,11 +2,13 @@ use super::prelude::*; #[test] fn nowrap() -> Result<()> { - let mut child = StdCommandWrap::with_new("cat", |command| { - command.stdin(Stdio::piped()).stdout(Stdio::piped()); - }) - .spawn()? - .into_inner(); + let mut child = unsafe { + StdCommandWrap::with_new("cat", |command| { + command.stdin(Stdio::piped()).stdout(Stdio::piped()); + }) + .spawn()? + .into_inner_child() + }; if let Some(mut din) = child.stdin.take() { din.write_all(b"hello")?; @@ -23,12 +25,14 @@ fn nowrap() -> Result<()> { #[test] fn process_group() -> Result<()> { - let mut child = StdCommandWrap::with_new("cat", |command| { - command.stdin(Stdio::piped()).stdout(Stdio::piped()); - }) - .wrap(ProcessGroup::leader()) - .spawn()? - .into_inner(); + let mut child = unsafe { + StdCommandWrap::with_new("cat", |command| { + command.stdin(Stdio::piped()).stdout(Stdio::piped()); + }) + .wrap(ProcessGroup::leader()) + .spawn()? + .into_inner_child() + }; if let Some(mut din) = child.stdin.take() { din.write_all(b"hello")?; @@ -45,12 +49,14 @@ fn process_group() -> Result<()> { #[test] fn process_session() -> Result<()> { - let mut child = StdCommandWrap::with_new("cat", |command| { - command.stdin(Stdio::piped()).stdout(Stdio::piped()); - }) - .wrap(ProcessSession) - .spawn()? - .into_inner(); + let mut child = unsafe { + StdCommandWrap::with_new("cat", |command| { + command.stdin(Stdio::piped()).stdout(Stdio::piped()); + }) + .wrap(ProcessSession) + .spawn()? + .into_inner_child() + }; if let Some(mut din) = child.stdin.take() { din.write_all(b"hello")?; From 37b2b15708f0d933e769c4bb569b3f3713d987c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fe=CC=81lix=20Saparelli?= Date: Sat, 28 Jun 2025 17:11:50 +1200 Subject: [PATCH 08/18] implement necessary things for std --- src/std/core.rs | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/src/std/core.rs b/src/std/core.rs index d6e63c0..910e028 100644 --- a/src/std/core.rs +++ b/src/std/core.rs @@ -205,11 +205,7 @@ pub trait StdChildWrapper: Any + std::fmt::Debug + Send { /// and unwrapped processes. #[cfg(unix)] fn signal(&self, sig: i32) -> Result<()> { - kill( - Pid::from_raw(i32::try_from(self.id()).map_err(std::io::Error::other)?), - Signal::try_from(sig)?, - ) - .map_err(std::io::Error::from) + self.inner().signal(sig) } } @@ -240,6 +236,23 @@ impl StdChildWrapper for StdChild { fn stderr(&mut self) -> &mut Option { &mut self.0.stderr } + fn id(&self) -> u32 { + self.0.id() + } + fn try_wait(&mut self) -> Result> { + self.0.try_wait() + } + fn wait(&mut self) -> Result { + self.0.wait() + } + #[cfg(unix)] + fn signal(&self, sig: i32) -> Result<()> { + kill( + Pid::from_raw(i32::try_from(self.id()).map_err(std::io::Error::other)?), + Signal::try_from(sig)?, + ) + .map_err(std::io::Error::from) + } } impl dyn StdChildWrapper { From ed88fc6d9d389e8dadf81ffb9b69d0a6a537c1d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fe=CC=81lix=20Saparelli?= Date: Sat, 28 Jun 2025 17:15:41 +1200 Subject: [PATCH 09/18] implement necessary things for tokio --- src/tokio/core.rs | 34 +++++++++++++++++++++++++--------- 1 file changed, 25 insertions(+), 9 deletions(-) diff --git a/src/tokio/core.rs b/src/tokio/core.rs index 49b3b86..9271a28 100644 --- a/src/tokio/core.rs +++ b/src/tokio/core.rs @@ -202,15 +202,7 @@ pub trait TokioChildWrapper: Any + std::fmt::Debug + Send { /// and unwrapped processes. #[cfg(unix)] fn signal(&self, sig: i32) -> Result<()> { - if let Some(id) = self.id() { - kill( - Pid::from_raw(i32::try_from(id).map_err(std::io::Error::other)?), - Signal::try_from(sig)?, - ) - .map_err(std::io::Error::from) - } else { - Ok(()) - } + self.inner().signal(sig) } } @@ -233,6 +225,30 @@ impl TokioChildWrapper for Child { fn stderr(&mut self) -> &mut Option { &mut self.stderr } + fn id(&self) -> Option { + self.id() + } + fn start_kill(&mut self) -> Result<()> { + self.start_kill() + } + fn try_wait(&mut self) -> Result> { + self.try_wait() + } + fn wait(&mut self) -> Pin> + Send + '_>> { + Box::pin(self.wait()) + } + #[cfg(unix)] + fn signal(&self, sig: i32) -> Result<()> { + if let Some(id) = self.id() { + kill( + Pid::from_raw(i32::try_from(id).map_err(std::io::Error::other)?), + Signal::try_from(sig)?, + ) + .map_err(std::io::Error::from) + } else { + Ok(()) + } + } } impl dyn TokioChildWrapper { From f32b5f507ab13cac39e894ab1769b524b46eec11 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fe=CC=81lix=20Saparelli?= Date: Sat, 28 Jun 2025 19:16:27 +1200 Subject: [PATCH 10/18] fix tests --- src/std/core.rs | 3 +-- tests/tokio_unix/kill_and_try_wait.rs | 2 +- tests/tokio_unix/multiproc_linux.rs | 4 ++-- tests/tokio_unix/wait_after_die.rs | 6 +++--- tests/tokio_unix/wait_twice.rs | 12 ++++++------ tests/tokio_unix/wait_twice_after_sigterm.rs | 12 ++++++------ 6 files changed, 19 insertions(+), 20 deletions(-) diff --git a/src/std/core.rs b/src/std/core.rs index 910e028..e5dbe4e 100644 --- a/src/std/core.rs +++ b/src/std/core.rs @@ -36,10 +36,9 @@ crate::generic_wrap::Wrap!( /// /// ```rust /// use process_wrap::std::*; -/// use std::process::Child; /// /// #[derive(Debug)] -/// pub struct YourChildWrapper(Child); +/// pub struct YourChildWrapper(StdChild); /// /// impl StdChildWrapper for YourChildWrapper { /// fn inner(&self) -> &dyn StdChildWrapper { diff --git a/tests/tokio_unix/kill_and_try_wait.rs b/tests/tokio_unix/kill_and_try_wait.rs index fce83ba..2c25466 100644 --- a/tests/tokio_unix/kill_and_try_wait.rs +++ b/tests/tokio_unix/kill_and_try_wait.rs @@ -29,7 +29,7 @@ async fn process_group() -> Result<()> { Box::into_pin(child.kill()).await?; - let status = Box::into_pin(child.wait()).await?; + let status = child.wait().await?; assert!(!status.success()); sleep(DIE_TIME).await; diff --git a/tests/tokio_unix/multiproc_linux.rs b/tests/tokio_unix/multiproc_linux.rs index 89dc029..fa41499 100644 --- a/tests/tokio_unix/multiproc_linux.rs +++ b/tests/tokio_unix/multiproc_linux.rs @@ -83,7 +83,7 @@ async fn process_group_kill_group() -> Result<()> { nix::sys::signal::killpg(nix::unistd::Pid::from_raw(parent), Signal::SIGKILL).unwrap(); sleep(DIE_TIME).await; assert!(!pid_alive(child), "child process should be dead"); - Box::into_pin(leader.wait()).await.unwrap(); + leader.wait().await.unwrap(); assert!(!pid_alive(parent), "parent process should be dead"); Ok(()) @@ -170,7 +170,7 @@ async fn process_session_kill_group() -> Result<()> { nix::sys::signal::killpg(nix::unistd::Pid::from_raw(parent), Signal::SIGKILL).unwrap(); sleep(DIE_TIME).await; assert!(!pid_alive(child), "child process should be dead"); - Box::into_pin(leader.wait()).await.unwrap(); + leader.wait().await.unwrap(); assert!(!pid_alive(parent), "parent process should be dead"); Ok(()) diff --git a/tests/tokio_unix/wait_after_die.rs b/tests/tokio_unix/wait_after_die.rs index c197cdf..f1c85f7 100644 --- a/tests/tokio_unix/wait_after_die.rs +++ b/tests/tokio_unix/wait_after_die.rs @@ -8,7 +8,7 @@ async fn nowrap() -> Result<()> { .spawn()?; sleep(DIE_TIME).await; - let status = Box::into_pin(child.wait()).await?; + let status = child.wait().await?; assert!(status.success()); Ok(()) @@ -23,7 +23,7 @@ async fn process_group() -> Result<()> { .spawn()?; sleep(DIE_TIME).await; - let status = Box::into_pin(child.wait()).await?; + let status = child.wait().await?; assert!(status.success()); Ok(()) @@ -38,7 +38,7 @@ async fn process_session() -> Result<()> { .spawn()?; sleep(DIE_TIME).await; - let status = Box::into_pin(child.wait()).await?; + let status = child.wait().await?; assert!(status.success()); Ok(()) diff --git a/tests/tokio_unix/wait_twice.rs b/tests/tokio_unix/wait_twice.rs index 40461f2..3560cb4 100644 --- a/tests/tokio_unix/wait_twice.rs +++ b/tests/tokio_unix/wait_twice.rs @@ -7,10 +7,10 @@ async fn nowrap() -> Result<()> { }) .spawn()?; - let status = Box::into_pin(child.wait()).await?; + let status = child.wait().await?; assert!(status.success()); - let status = Box::into_pin(child.wait()).await?; + let status = child.wait().await?; assert!(status.success()); Ok(()) @@ -24,10 +24,10 @@ async fn process_group() -> Result<()> { .wrap(ProcessGroup::leader()) .spawn()?; - let status = Box::into_pin(child.wait()).await?; + let status = child.wait().await?; assert!(status.success()); - let status = Box::into_pin(child.wait()).await?; + let status = child.wait().await?; assert!(status.success()); Ok(()) @@ -41,10 +41,10 @@ async fn process_session() -> Result<()> { .wrap(ProcessSession) .spawn()?; - let status = Box::into_pin(child.wait()).await?; + let status = child.wait().await?; assert!(status.success()); - let status = Box::into_pin(child.wait()).await?; + let status = child.wait().await?; assert!(status.success()); Ok(()) diff --git a/tests/tokio_unix/wait_twice_after_sigterm.rs b/tests/tokio_unix/wait_twice_after_sigterm.rs index 2b2e7d7..e72b4a9 100644 --- a/tests/tokio_unix/wait_twice_after_sigterm.rs +++ b/tests/tokio_unix/wait_twice_after_sigterm.rs @@ -10,10 +10,10 @@ async fn nowrap() -> Result<()> { child.signal(Signal::SIGTERM as _)?; - let status = Box::into_pin(child.wait()).await?; + let status = child.wait().await?; assert_eq!(status.signal(), Some(Signal::SIGTERM as i32), "wait() one"); - let status = Box::into_pin(child.wait()).await?; + let status = child.wait().await?; assert_eq!(status.signal(), Some(Signal::SIGTERM as i32), "wait() two"); Ok(()) @@ -30,10 +30,10 @@ async fn process_group() -> Result<()> { child.signal(Signal::SIGTERM as _)?; - let status = Box::into_pin(child.wait()).await?; + let status = child.wait().await?; assert_eq!(status.signal(), Some(Signal::SIGTERM as i32), "wait() one"); - let status = Box::into_pin(child.wait()).await?; + let status = child.wait().await?; assert_eq!(status.signal(), Some(Signal::SIGTERM as i32), "wait() two"); Ok(()) @@ -50,10 +50,10 @@ async fn process_session() -> Result<()> { child.signal(Signal::SIGTERM as _)?; - let status = Box::into_pin(child.wait()).await?; + let status = child.wait().await?; assert_eq!(status.signal(), Some(Signal::SIGTERM as i32), "wait() one"); - let status = Box::into_pin(child.wait()).await?; + let status = child.wait().await?; assert_eq!(status.signal(), Some(Signal::SIGTERM as i32), "wait() two"); Ok(()) From 670725561b31406ad6a2cdf263f8d017e39f3575 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fe=CC=81lix=20Saparelli?= Date: Sat, 28 Jun 2025 19:19:11 +1200 Subject: [PATCH 11/18] fix windows tests --- src/std/job_object.rs | 4 ++-- tests/std_windows/into_inner_write_stdin.rs | 8 ++++---- tests/tokio_windows/wait_after_die.rs | 4 ++-- tests/tokio_windows/wait_twice.rs | 8 ++++---- 4 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/std/job_object.rs b/src/std/job_object.rs index 8c7bc47..f557e97 100644 --- a/src/std/job_object.rs +++ b/src/std/job_object.rs @@ -135,13 +135,13 @@ impl StdChildWrapper for JobObjectChild { let JobPort { completion_port, .. } = self.job_port; - wait_on_job(completion_port, None)?; + let _ = wait_on_job(completion_port, None)?; Ok(status) } #[cfg_attr(feature = "tracing", instrument(level = "debug", skip(self)))] fn try_wait(&mut self) -> Result> { - wait_on_job(self.job_port.completion_port, Some(Duration::ZERO))?; + let _ = wait_on_job(self.job_port.completion_port, Some(Duration::ZERO))?; self.inner.try_wait() } } diff --git a/tests/std_windows/into_inner_write_stdin.rs b/tests/std_windows/into_inner_write_stdin.rs index ff7ade9..d19f74d 100644 --- a/tests/std_windows/into_inner_write_stdin.rs +++ b/tests/std_windows/into_inner_write_stdin.rs @@ -11,12 +11,12 @@ fn nowrap() -> Result<()> { .spawn()? .into_inner(); - if let Some(mut din) = child.stdin.take() { + if let Some(mut din) = child.stdin().take() { din.write_all(b"hello")?; } let mut output = String::new(); - if let Some(mut out) = child.stdout.take() { + if let Some(mut out) = child.stdout().take() { out.read_to_string(&mut output)?; } @@ -36,12 +36,12 @@ fn job_object() -> Result<()> { .spawn()? .into_inner(); - if let Some(mut din) = child.stdin.take() { + if let Some(mut din) = child.stdin().take() { din.write_all(b"hello")?; } let mut output = String::new(); - if let Some(mut out) = child.stdout.take() { + if let Some(mut out) = child.stdout().take() { out.read_to_string(&mut output)?; } diff --git a/tests/tokio_windows/wait_after_die.rs b/tests/tokio_windows/wait_after_die.rs index 2a91f7d..0b1f840 100644 --- a/tests/tokio_windows/wait_after_die.rs +++ b/tests/tokio_windows/wait_after_die.rs @@ -8,7 +8,7 @@ async fn nowrap() -> Result<()> { .spawn()?; sleep(DIE_TIME).await; - let status = Box::into_pin(child.wait()).await?; + let status = child.wait().await?; assert!(status.success()); Ok(()) @@ -23,7 +23,7 @@ async fn job_object() -> Result<()> { .spawn()?; sleep(DIE_TIME).await; - let status = Box::into_pin(child.wait()).await?; + let status = child.wait().await?; assert!(status.success()); Ok(()) diff --git a/tests/tokio_windows/wait_twice.rs b/tests/tokio_windows/wait_twice.rs index a89c7f4..c79e9e6 100644 --- a/tests/tokio_windows/wait_twice.rs +++ b/tests/tokio_windows/wait_twice.rs @@ -7,10 +7,10 @@ async fn nowrap() -> Result<()> { }) .spawn()?; - let status = Box::into_pin(child.wait()).await?; + let status = child.wait().await?; assert!(status.success()); - let status = Box::into_pin(child.wait()).await?; + let status = child.wait().await?; assert!(status.success()); Ok(()) @@ -24,10 +24,10 @@ async fn job_object() -> Result<()> { .wrap(JobObject) .spawn()?; - let status = Box::into_pin(child.wait()).await?; + let status = child.wait().await?; assert!(status.success()); - let status = Box::into_pin(child.wait()).await?; + let status = child.wait().await?; assert!(status.success()); Ok(()) From f9cc613628bfad81c4f30fd1078857689c7c982a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fe=CC=81lix=20Saparelli?= Date: Sat, 28 Jun 2025 21:09:33 +1200 Subject: [PATCH 12/18] no longer need StdChild wrapper --- src/std.rs | 2 +- src/std/core.rs | 48 +++++++++++++++++++----------------------------- 2 files changed, 20 insertions(+), 30 deletions(-) diff --git a/src/std.rs b/src/std.rs index 89d15d3..f3e30fd 100644 --- a/src/std.rs +++ b/src/std.rs @@ -9,7 +9,7 @@ //! ``` #[doc(inline)] -pub use core::{StdChild, StdChildWrapper, StdCommandWrap, StdCommandWrapper}; +pub use core::{StdChildWrapper, StdCommandWrap, StdCommandWrapper}; #[cfg(all(windows, feature = "creation-flags"))] #[doc(inline)] pub use creation_flags::CreationFlags; diff --git a/src/std/core.rs b/src/std/core.rs index e5dbe4e..3de386a 100644 --- a/src/std/core.rs +++ b/src/std/core.rs @@ -16,29 +16,30 @@ crate::generic_wrap::Wrap!( StdCommandWrapper, Child, StdChildWrapper, - StdChild // |child| StdChild(child) + |child| child ); /// Wrapper for `std::process::Child`. /// /// This trait exposes most of the functionality of the underlying [`Child`]. It is implemented for -/// [`StdChild`] (a thin wrapper around [`Child`]) (because implementing directly on [`Child`] would -/// loop) and by wrappers. +/// [`Child`] and by wrappers. /// /// The required methods are `inner`, `inner_mut`, and `into_inner`. That provides access to the -/// underlying `Child` and allows the wrapper to be dropped and the `Child` to be used directly if -/// necessary. +/// lower layer and ultimately allows the wrappers to be unwrap and the `Child` to be used directly +/// if necessary. There are convenience `inner_child`, `inner_child_mut` and `into_inner_child` +/// methods on the trait object. /// /// It also makes it possible for all the other methods to have default implementations. Some are -/// direct passthroughs to the underlying `Child`, while others are more complex. +/// direct passthroughs to the lower layers, while others are more complex. /// /// Here's a simple example of a wrapper: /// /// ```rust /// use process_wrap::std::*; +/// use std::process::Child; /// /// #[derive(Debug)] -/// pub struct YourChildWrapper(StdChild); +/// pub struct YourChildWrapper(Child); /// /// impl StdChildWrapper for YourChildWrapper { /// fn inner(&self) -> &dyn StdChildWrapper { @@ -208,15 +209,7 @@ pub trait StdChildWrapper: Any + std::fmt::Debug + Send { } } -/// A thin wrapper around [`Child`]. -/// -/// This is used only because implementing [`StdChildWrapper`] directly on std's [`Child`] creates -/// loops in the type system. It is not intended to be used directly, but only to be used internally -/// by the library. -#[derive(Debug)] -pub struct StdChild(pub Child); - -impl StdChildWrapper for StdChild { +impl StdChildWrapper for Child { fn inner(&self) -> &dyn StdChildWrapper { self } @@ -227,22 +220,22 @@ impl StdChildWrapper for StdChild { self } fn stdin(&mut self) -> &mut Option { - &mut self.0.stdin + &mut self.stdin } fn stdout(&mut self) -> &mut Option { - &mut self.0.stdout + &mut self.stdout } fn stderr(&mut self) -> &mut Option { - &mut self.0.stderr + &mut self.stderr } fn id(&self) -> u32 { - self.0.id() + self.id() } fn try_wait(&mut self) -> Result> { - self.0.try_wait() + self.try_wait() } fn wait(&mut self) -> Result { - self.0.wait() + self.wait() } #[cfg(unix)] fn signal(&self, sig: i32) -> Result<()> { @@ -260,7 +253,7 @@ impl dyn StdChildWrapper { } fn is_raw_child(&self) -> bool { - self.downcast_ref::().is_some() + self.downcast_ref::().is_some() } /// Obtain a reference to the underlying [`Child`]. @@ -271,7 +264,7 @@ impl dyn StdChildWrapper { } // UNWRAP: we've just checked that it's Some with is_raw_child() - &inner.downcast_ref::().unwrap().0 + inner.downcast_ref().unwrap() } /// Obtain a mutable reference to the underlying [`Child`]. @@ -284,10 +277,7 @@ impl dyn StdChildWrapper { } // UNWRAP: we've just checked that with is_raw_child() - &mut (inner as &mut dyn Any) - .downcast_mut::() - .unwrap() - .0 + (inner as &mut dyn Any).downcast_mut().unwrap() } /// Obtain the underlying [`Child`]. @@ -300,7 +290,7 @@ impl dyn StdChildWrapper { } // UNWRAP: we've just checked that with is_raw_child() - (inner as Box).downcast::().unwrap().0 + *(inner as Box).downcast().unwrap() } } From 651cc09d324490af78b80b516835e318b55d08a3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fe=CC=81lix=20Saparelli?= Date: Sat, 28 Jun 2025 21:31:19 +1200 Subject: [PATCH 13/18] extra parens? --- tests/std_windows/kill_and_try_wait.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/std_windows/kill_and_try_wait.rs b/tests/std_windows/kill_and_try_wait.rs index 805bede..a8b8c67 100644 --- a/tests/std_windows/kill_and_try_wait.rs +++ b/tests/std_windows/kill_and_try_wait.rs @@ -8,7 +8,7 @@ fn nowrap() -> Result<()> { .spawn()?; assert!(child.try_wait()?.is_none(), "pre kill"); - (child.kill())?; + child.kill()?; sleep(DIE_TIME); assert!(child.try_wait()?.is_some(), "try_wait() one"); @@ -27,7 +27,7 @@ fn job_object() -> Result<()> { .spawn()?; assert!(child.try_wait()?.is_none(), "pre kill"); - (child.kill())?; + child.kill()?; sleep(DIE_TIME); assert!(child.try_wait()?.is_some(), "try_wait() one"); From 74929bdea55c9e7afbbc2de18e18e82ed4462127 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fe=CC=81lix=20Saparelli?= Date: Sat, 28 Jun 2025 22:04:30 +1200 Subject: [PATCH 14/18] fix loop --- src/std/core.rs | 6 +++--- src/tokio/core.rs | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/std/core.rs b/src/std/core.rs index 3de386a..7ee98f1 100644 --- a/src/std/core.rs +++ b/src/std/core.rs @@ -229,13 +229,13 @@ impl StdChildWrapper for Child { &mut self.stderr } fn id(&self) -> u32 { - self.id() + Child::id(self) } fn try_wait(&mut self) -> Result> { - self.try_wait() + Child::try_wait(self) } fn wait(&mut self) -> Result { - self.wait() + Child::wait(self) } #[cfg(unix)] fn signal(&self, sig: i32) -> Result<()> { diff --git a/src/tokio/core.rs b/src/tokio/core.rs index 9271a28..e3d232b 100644 --- a/src/tokio/core.rs +++ b/src/tokio/core.rs @@ -226,16 +226,16 @@ impl TokioChildWrapper for Child { &mut self.stderr } fn id(&self) -> Option { - self.id() + Child::id(self) } fn start_kill(&mut self) -> Result<()> { - self.start_kill() + Child::start_kill(self) } fn try_wait(&mut self) -> Result> { - self.try_wait() + Child::try_wait(self) } fn wait(&mut self) -> Pin> + Send + '_>> { - Box::pin(self.wait()) + Box::pin(Child::wait(self)) } #[cfg(unix)] fn signal(&self, sig: i32) -> Result<()> { From 1fc90cbdf3af0d5aa3661570960244130c6a065c Mon Sep 17 00:00:00 2001 From: Brooks J Rady Date: Sat, 28 Jun 2025 23:23:21 +0100 Subject: [PATCH 15/18] fix(std): resolve stack overflow on Windows --- src/std/core.rs | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/std/core.rs b/src/std/core.rs index 7ee98f1..86e0f59 100644 --- a/src/std/core.rs +++ b/src/std/core.rs @@ -127,15 +127,7 @@ pub trait StdChildWrapper: Any + std::fmt::Debug + Send { /// library uses it to provide a consistent API across both std and Tokio (and because it's a /// generally useful API). fn start_kill(&mut self) -> Result<()> { - #[cfg(unix)] - { - self.signal(Signal::SIGKILL as _) - } - - #[cfg(not(unix))] - { - self.inner_mut().kill() - } + self.inner_mut().start_kill() } /// Check if the `Child` has exited without blocking, and if so, return its exit status. @@ -231,6 +223,17 @@ impl StdChildWrapper for Child { fn id(&self) -> u32 { Child::id(self) } + fn start_kill(&mut self) -> Result<()> { + #[cfg(unix)] + { + self.signal(Signal::SIGKILL as _) + } + + #[cfg(not(unix))] + { + Child::kill(self) + } + } fn try_wait(&mut self) -> Result> { Child::try_wait(self) } From a19519ecd506c3c856f136df3669ad12e28382b5 Mon Sep 17 00:00:00 2001 From: Brooks J Rady Date: Sun, 29 Jun 2025 14:20:56 +0100 Subject: [PATCH 16/18] feat(api): add `&mut Command` to `pre_spawn()` This addition makes it possible to mutate the `Command` in `pre_spawn()`, spawn a `Child` from that mutated command, then roll-back the changes to command in `post_spawn()`. --- src/generic_wrap.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/generic_wrap.rs b/src/generic_wrap.rs index ce9c393..a2fe52a 100644 --- a/src/generic_wrap.rs +++ b/src/generic_wrap.rs @@ -91,7 +91,7 @@ macro_rules! Wrap { for (id, wrapper) in wrappers.iter_mut() { #[cfg(feature = "tracing")] ::tracing::debug!(?id, "post_spawn"); - wrapper.post_spawn(&mut child, self)?; + wrapper.post_spawn(command, &mut child, self)?; } let mut child = Box::new( @@ -204,7 +204,7 @@ macro_rules! Wrap { /// how `CreationFlags` on Windows works along with `JobObject`. /// /// Default: no-op. - fn post_spawn(&mut self, _child: &mut $child, _core: &$name) -> Result<()> { + fn post_spawn(&mut self, _command: &mut $command, _child: &mut $child, _core: &$name) -> Result<()> { Ok(()) } From ed76262ba043d12b5f14845d5d49fa47961da241 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?F=C3=A9lix=20Saparelli?= Date: Mon, 30 Jun 2025 12:00:54 +1200 Subject: [PATCH 17/18] fix stack overflow on windows thanks to @thelostlambda --- src/std/core.rs | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/std/core.rs b/src/std/core.rs index 7ee98f1..86e0f59 100644 --- a/src/std/core.rs +++ b/src/std/core.rs @@ -127,15 +127,7 @@ pub trait StdChildWrapper: Any + std::fmt::Debug + Send { /// library uses it to provide a consistent API across both std and Tokio (and because it's a /// generally useful API). fn start_kill(&mut self) -> Result<()> { - #[cfg(unix)] - { - self.signal(Signal::SIGKILL as _) - } - - #[cfg(not(unix))] - { - self.inner_mut().kill() - } + self.inner_mut().start_kill() } /// Check if the `Child` has exited without blocking, and if so, return its exit status. @@ -231,6 +223,17 @@ impl StdChildWrapper for Child { fn id(&self) -> u32 { Child::id(self) } + fn start_kill(&mut self) -> Result<()> { + #[cfg(unix)] + { + self.signal(Signal::SIGKILL as _) + } + + #[cfg(not(unix))] + { + Child::kill(self) + } + } fn try_wait(&mut self) -> Result> { Child::try_wait(self) } From e64288bce91043c5452e1821be9873bbff02eed0 Mon Sep 17 00:00:00 2001 From: Brooks J Rady Date: Tue, 22 Jul 2025 14:52:23 +0100 Subject: [PATCH 18/18] docs: add doctests for an example logging wrapper --- Cargo.lock | 40 +++++++- Cargo.toml | 1 + src/lib.rs | 284 ++++++++++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 323 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index e967f86..5786a67 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1,6 +1,6 @@ # This file is automatically @generated by Cargo. # It is not intended for manual editing. -version = 3 +version = 4 [[package]] name = "addr2line" @@ -176,6 +176,12 @@ version = "0.3.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "2acce4a10f12dc2fb14a218589d4f1f62ef011b2d0cc4b3cb1bba8e94da14649" +[[package]] +name = "fastrand" +version = "2.3.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "37909eebbb50d72f9059c3b6d82c0463f2ff062c9e95845c43a6c9c0355411be" + [[package]] name = "flate2" version = "1.1.1" @@ -360,6 +366,12 @@ dependencies = [ "libc", ] +[[package]] +name = "linux-raw-sys" +version = "0.9.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "cd945864f07fe9f5371a27ad7b52a172b4b499999f1d97574c9fa68373937e12" + [[package]] name = "log" version = "0.4.27" @@ -540,6 +552,7 @@ dependencies = [ "indexmap", "nix 0.30.1", "remoteprocess", + "tempfile", "tokio", "tracing", "windows", @@ -631,6 +644,19 @@ version = "1.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "08d43f7aa6b08d49f382cde6a7982047c3426db949b1424bc4b7ec9ae12c6ce2" +[[package]] +name = "rustix" +version = "1.0.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "c71e83d6afe7ff64890ec6b71d6a69bb8a610ab78ce364b3352876bb4c801266" +dependencies = [ + "bitflags 2.9.1", + "errno", + "libc", + "linux-raw-sys", + "windows-sys 0.59.0", +] + [[package]] name = "ruzstd" version = "0.7.3" @@ -713,6 +739,18 @@ dependencies = [ "unicode-ident", ] +[[package]] +name = "tempfile" +version = "3.20.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e8a64e3985349f2441a1a9ef0b853f869006c3855f2cda6862a94d26ebb9d6a1" +dependencies = [ + "fastrand", + "once_cell", + "rustix", + "windows-sys 0.59.0", +] + [[package]] name = "tokio" version = "1.45.0" diff --git a/Cargo.toml b/Cargo.toml index 920483d..83a3f02 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -36,6 +36,7 @@ windows = { version = "0.61.1", optional = true } [dev-dependencies] remoteprocess = "0.5.0" +tempfile = { version = "3.20.0", default-features = false } tokio = { version = "1.38.2", features = ["io-util", "macros", "process", "rt", "rt-multi-thread", "time"] } [features] diff --git a/src/lib.rs b/src/lib.rs index 666c45e..eddc115 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -9,7 +9,6 @@ //! //! ```rust,no_run //! # fn main() -> std::io::Result<()> { -//! use std::process::Command; //! use process_wrap::std::*; //! //! let mut command = StdCommandWrap::with_new("watch", |command| { command.arg("ls"); }); @@ -150,6 +149,289 @@ //! If your functionality is order-dependent, make sure to specify so in your documentation! By //! default does nothing: no wrapping is performed and the input `child` is returned as-is. //! +//! ## An Example Logging Wrapper +//! +//! Let's implement a logging wrapper that redirects a `Command`'s `stdout` and `stderr` into a +//! text file. We can use `std::io::pipe` to merge `stdout` and `stderr` into one channel, then +//! `std::io::copy` in a background thread to non-blockingly stream that data to disk as it comes +//! in. +//! +//! ```rust +//! # use process_wrap::std::{StdCommandWrap, StdCommandWrapper}; +//! # use std::{fs::File, io, path::PathBuf, process::Command, thread}; +//! #[derive(Debug)] +//! struct LogFile { +//! path: PathBuf, +//! } +//! +//! impl LogFile { +//! fn new(path: impl Into) -> Self { +//! Self { path: path.into() } +//! } +//! } +//! +//! impl StdCommandWrapper for LogFile { +//! fn pre_spawn(&mut self, command: &mut Command, _core: &StdCommandWrap) -> io::Result<()> { +//! let mut logfile = File::create(&self.path)?; +//! let (mut rx, tx) = io::pipe()?; +//! +//! thread::spawn(move || { +//! io::copy(&mut rx, &mut logfile).unwrap(); +//! }); +//! +//! command.stdout(tx.try_clone()?).stderr(tx); +//! Ok(()) +//! } +//! } +//! ``` +//! +//! That's a great start, but it's actually introduced a resource leak: if the main thread of your +//! program exits before that background one does, then the background thread won't get a chance to +//! call `logfile`'s `Drop` implementation which closes the file. The file handle will be left open! +//! To fix this, we'll need to keep track of the background thread's `ThreadHandle` and `.join()` it +//! when calling `.wait()` on the `ChildWrapper`. +//! +//! ```rust +//! # use process_wrap::std::{StdChildWrapper, StdCommandWrap, StdCommandWrapper}; +//! # use std::{ +//! # fs::File, +//! # io, mem, +//! # path::PathBuf, +//! # process::{Command, ExitStatus}, +//! # thread::{self, JoinHandle}, +//! # }; +//! #[derive(Debug)] +//! struct LogFile { +//! path: PathBuf, +//! thread: Option>, +//! } +//! +//! impl LogFile { +//! fn new(path: impl Into) -> Self { +//! Self { +//! path: path.into(), +//! thread: None, +//! } +//! } +//! } +//! +//! impl StdCommandWrapper for LogFile { +//! fn pre_spawn(&mut self, command: &mut Command, _core: &StdCommandWrap) -> io::Result<()> { +//! let mut logfile = File::create(&self.path)?; +//! let (mut rx, tx) = io::pipe()?; +//! +//! self.thread = Some(thread::spawn(move || { +//! io::copy(&mut rx, &mut logfile).unwrap(); +//! })); +//! +//! command.stdout(tx.try_clone()?).stderr(tx); +//! Ok(()) +//! } +//! +//! fn wrap_child( +//! &mut self, +//! child: Box, +//! _core: &StdCommandWrap, +//! ) -> io::Result> { +//! let wrapped_child = LogFileChild { +//! inner: child, +//! thread: mem::take(&mut self.thread), +//! }; +//! Ok(Box::new(wrapped_child)) +//! } +//! } +//! +//! #[derive(Debug)] +//! struct LogFileChild { +//! inner: Box, +//! thread: Option>, +//! } +//! +//! impl StdChildWrapper for LogFileChild { +//! fn inner(&self) -> &dyn StdChildWrapper { +//! &*self.inner +//! } +//! +//! fn inner_mut(&mut self) -> &mut dyn StdChildWrapper { +//! &mut *self.inner +//! } +//! +//! fn into_inner(self: Box) -> Box { +//! self.inner +//! } +//! +//! fn wait(&mut self) -> io::Result { +//! let exit_status = self.inner.wait(); +//! +//! if let Some(thread) = mem::take(&mut self.thread) { +//! thread.join().unwrap(); +//! } +//! +//! exit_status +//! } +//! } +//! ``` +//! +//! Now we're cleaning up after ourselves, but there is one last issue: if you actually call +//! `.wait()`, then your program will deadlock! This is because `io::copy` copies data until `rx` +//! returns an EOF, but that only happens after *all* copies of `tx` are dropped. Currently, our +//! `Command` is holding onto `tx` even after calling `.spawn()`, so unless we manually drop the +//! `Command` (freeing both copies of `tx`) before calling `.wait()`, our program will deadlock! +//! We can fix this by telling `Command` to drop `tx` right after spawning the child — by this +//! point, the `ChildWrapper` will have already inherited the copies of `tx` that it needs, so +//! dropping `tx` from `Command` should be totally safe. We'll get `Command` to "drop" `tx` by +//! setting its `stdin` and `stdout` to `Stdio::null()` in `CommandWrapper::post_spawn()`. +//! +//! ```rust +//! # use process_wrap::std::{StdCommandWrap, StdCommandWrapper}; +//! # use std::{ +//! # io, +//! # path::PathBuf, +//! # process::{Child, Command, Stdio}, +//! # thread::JoinHandle, +//! # }; +//! # #[derive(Debug)] +//! # struct LogFile { +//! # path: PathBuf, +//! # thread: Option>, +//! # } +//! # +//! impl StdCommandWrapper for LogFile { +//! // ... snip ... +//! fn post_spawn( +//! &mut self, +//! command: &mut Command, +//! _child: &mut Child, +//! _core: &StdCommandWrap, +//! ) -> io::Result<()> { +//! command.stdout(Stdio::null()).stderr(Stdio::null()); +//! +//! Ok(()) +//! } +//! // ... snip ... +//! } +//! ``` +//! +//! Finally, we can test that our new command-wrapper works: +//! +//! ```rust +//! # use process_wrap::std::{StdChildWrapper, StdCommandWrap, StdCommandWrapper}; +//! # use std::{ +//! # error::Error, +//! # fs::{self, File}, +//! # io, mem, +//! # path::PathBuf, +//! # process::{Child, Command, ExitStatus, Stdio}, +//! # thread::{self, JoinHandle}, +//! # }; +//! # use tempfile::NamedTempFile; +//! # #[derive(Debug)] +//! # struct LogFile { +//! # path: PathBuf, +//! # thread: Option>, +//! # } +//! # +//! # impl LogFile { +//! # fn new(path: impl Into) -> Self { +//! # Self { +//! # path: path.into(), +//! # thread: None, +//! # } +//! # } +//! # } +//! # +//! # impl StdCommandWrapper for LogFile { +//! # fn pre_spawn(&mut self, command: &mut Command, _core: &StdCommandWrap) -> io::Result<()> { +//! # let mut logfile = File::create(&self.path)?; +//! # let (mut rx, tx) = io::pipe()?; +//! # +//! # self.thread = Some(thread::spawn(move || { +//! # io::copy(&mut rx, &mut logfile).unwrap(); +//! # })); +//! # +//! # command.stdout(tx.try_clone()?).stderr(tx); +//! # Ok(()) +//! # } +//! # +//! # fn post_spawn( +//! # &mut self, +//! # command: &mut Command, +//! # _child: &mut Child, +//! # _core: &StdCommandWrap, +//! # ) -> io::Result<()> { +//! # command.stdout(Stdio::null()).stderr(Stdio::null()); +//! # +//! # Ok(()) +//! # } +//! # +//! # fn wrap_child( +//! # &mut self, +//! # child: Box, +//! # _core: &StdCommandWrap, +//! # ) -> io::Result> { +//! # let wrapped_child = LogFileChild { +//! # inner: child, +//! # thread: mem::take(&mut self.thread), +//! # }; +//! # Ok(Box::new(wrapped_child)) +//! # } +//! # } +//! # +//! # #[derive(Debug)] +//! # struct LogFileChild { +//! # inner: Box, +//! # thread: Option>, +//! # } +//! # +//! # impl StdChildWrapper for LogFileChild { +//! # fn inner(&self) -> &dyn StdChildWrapper { +//! # &*self.inner +//! # } +//! # +//! # fn inner_mut(&mut self) -> &mut dyn StdChildWrapper { +//! # &mut *self.inner +//! # } +//! # +//! # fn into_inner(self: Box) -> Box { +//! # self.inner +//! # } +//! # +//! # fn wait(&mut self) -> io::Result { +//! # let exit_status = self.inner.wait(); +//! # +//! # if let Some(thread) = mem::take(&mut self.thread) { +//! # thread.join().unwrap(); +//! # } +//! # +//! # exit_status +//! # } +//! # } +//! # +//! fn main() -> Result<(), Box> { +//! #[cfg(windows)] +//! let mut command = StdCommandWrap::with_new("cmd", |command| { +//! command.args(["/c", "echo Hello && echo World 1>&2"]); +//! }); +//! #[cfg(unix)] +//! let mut command = StdCommandWrap::with_new("sh", |command| { +//! command.args(["-c", "echo Hello && echo World 1>&2"]); +//! }); +//! +//! let logfile = NamedTempFile::new()?; +//! let logfile_path = logfile.path(); +//! +//! command.wrap(LogFile::new(logfile_path)).spawn()?.wait()?; +//! +//! let logfile_lines: Vec = fs::read_to_string(logfile_path)? +//! .lines() +//! .map(|l| l.trim().into()) +//! .collect(); +//! assert_eq!(logfile_lines, vec!["Hello", "World"]); +//! +//! Ok(()) +//! } +//! ``` +//! //! # Features //! //! ## Frontends