feat: change child wrapper inner()s to return one layer down (#23)

Author: passcod

.github/workflows/test.yml

@@ -16,7 +16,7 @@ jobs:
           - windows
         toolchain:
           - stable
-          - 1.77.0
+          - 1.86.0
         features:
           - tokio1
           - std

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 }
@@ -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"]
 

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.
 

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;

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;

src/std/core.rs

@@ -1,4 +1,5 @@
 use std::{
+	any::Any,
 	io::{Read, Result},
 	process::{Child, ChildStderr, ChildStdin, ChildStdout, Command, ExitStatus, Output},
 };
@@ -15,22 +16,21 @@ crate::generic_wrap::Wrap!(
 	StdCommandWrapper,
 	Child,
 	StdChildWrapper,
-	StdChild // |child| StdChild(child)
+	|child| 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
-/// [`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:
 ///
@@ -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<Self>) -> Child {
-///         (*self).0
+///     fn into_inner(self: Box<Self>) -> Box<dyn StdChildWrapper> {
+///         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<Self>) -> 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<Self>) -> Box<dyn StdChildWrapper>;
 
 	/// 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<ChildStdin> {
-		&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<ChildStdout> {
-		&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<ChildStderr> {
-		&mut self.inner_mut().stderr
+		self.inner_mut().stderr()
 	}
 
 	/// Obtain the `Child`'s process ID.
@@ -127,15 +127,7 @@ pub trait StdChildWrapper {
 	/// 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.
@@ -204,6 +196,51 @@ pub trait StdChildWrapper {
 	/// was introduced by command-group to abstract over the signal behaviour between process groups
 	/// and unwrapped processes.
 	#[cfg(unix)]
+	fn signal(&self, sig: i32) -> Result<()> {
+		self.inner().signal(sig)
+	}
+}
+
+impl StdChildWrapper for Child {
+	fn inner(&self) -> &dyn StdChildWrapper {
+		self
+	}
+	fn inner_mut(&mut self) -> &mut dyn StdChildWrapper {
+		self
+	}
+	fn into_inner(self: Box<Self>) -> Box<dyn StdChildWrapper> {
+		self
+	}
+	fn stdin(&mut self) -> &mut Option<ChildStdin> {
+		&mut self.stdin
+	}
+	fn stdout(&mut self) -> &mut Option<ChildStdout> {
+		&mut self.stdout
+	}
+	fn stderr(&mut self) -> &mut Option<ChildStderr> {
+		&mut self.stderr
+	}
+	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<Option<ExitStatus>> {
+		Child::try_wait(self)
+	}
+	fn wait(&mut self) -> Result<ExitStatus> {
+		Child::wait(self)
+	}
+	#[cfg(unix)]
 	fn signal(&self, sig: i32) -> Result<()> {
 		kill(
 			Pid::from_raw(i32::try_from(self.id()).map_err(std::io::Error::other)?),
@@ -212,25 +249,51 @@ pub trait StdChildWrapper {
 		.map_err(std::io::Error::from)
 	}
 }
-}
 
-/// 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 {
-	fn inner(&self) -> &Child {
-		&self.0
+impl dyn StdChildWrapper {
+	fn downcast_ref<T: 'static>(&self) -> Option<&T> {
+		(self as &dyn Any).downcast_ref()
 	}
-	fn inner_mut(&mut self) -> &mut Child {
-		&mut self.0
+
+	fn is_raw_child(&self) -> bool {
+		self.downcast_ref::<Child>().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()
 	}
-	fn into_inner(self: Box<Self>) -> Child {
-		(*self).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<Self>) -> 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<dyn Any>).downcast().unwrap()
 	}
 }
 

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<Self>) -> Child {
+	fn into_inner(self: Box<Self>) -> Box<dyn StdChildWrapper> {
 		// manually drop the completion port
 		let its = std::mem::ManuallyDrop::new(self.job_port);
 		unsafe { CloseHandle(its.completion_port.0) }.ok();
@@ -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<Option<ExitStatus>> {
-		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()
 	}
 }