diff --git a/.rust-mutants.toml b/.rust-mutants.toml index 44a9aa9..2e08f09 100644 --- a/.rust-mutants.toml +++ b/.rust-mutants.toml @@ -60,9 +60,9 @@ outcome = "survived" [[mutation.expect]] path = "src/transaction.rs" -item = "SpawnTransaction::create" +item = "attribute_list" rule = "condition-to-false" -original = "attribute_count == 0" +original = "count == 0" reason = "Equivalence: CreateProcessW accepts an empty attribute list." outcome = "survived" @@ -94,20 +94,20 @@ outcome = "survived" [[mutation.skip]] path = "src/sys.rs" -lines = "1210-1210" +lines = "1246-1246" reason = "Unreachable: GetEnvironmentStringsW returns null only when out of memory." [[mutation.skip]] path = "src/sys.rs" -lines = "1219-1219" +lines = "1255-1255" reason = "Hang-only: continuing past the block terminator loops forever." [[mutation.skip]] path = "src/transaction.rs" -lines = "572-572" +lines = "565-565" reason = "Unreachable: GetFullPathNameW cannot return a double quote." [[mutation.skip]] path = "src/transaction.rs" -lines = "127-131" -reason = "Unreachable: wide_nul fails only on an interior NUL, which validate_command rejects first; rejects_every_malformed_text_component holds it." +lines = "77-77" +reason = "Unreachable: current_directory fails only on an interior NUL, which validate_command rejects first; rejects_every_malformed_text_component holds it." diff --git a/deny.toml b/deny.toml index d651535..7ffbb89 100644 --- a/deny.toml +++ b/deny.toml @@ -33,6 +33,24 @@ multiple-versions-include-dev = true skip = [] skip-tree = [] wildcards = "deny" +# ADR 0016: no erased errors, erased dispatch, runtime registration, or implicit global state. +deny = [ + { crate = "anyhow", reason = "erases error types" }, + { crate = "arc-swap", reason = "shares mutable global state" }, + { crate = "async-trait", reason = "erases futures behind trait objects" }, + { crate = "color-eyre", reason = "erases error types" }, + { crate = "dashmap", reason = "shares mutable state and orders by hash" }, + { crate = "downcast-rs", reason = "dispatches on runtime types" }, + { crate = "erased-serde", reason = "erases serialization behind trait objects" }, + { crate = "eyre", reason = "erases error types" }, + { crate = "inventory", reason = "registers items at run time" }, + { crate = "lazy_static", reason = "initializes global state implicitly" }, + { crate = "linkme", reason = "registers items at link time" }, + { crate = "once_cell", reason = "initializes global state implicitly" }, + { crate = "parking_lot", reason = "shares mutable state behind locks" }, + { crate = "serde_traitobject", reason = "serializes trait objects" }, + { crate = "typetag", reason = "registers trait-object types at run time" }, +] [sources] allow-git = [] diff --git a/docs/adr/0007-spawn-transaction.md b/docs/adr/0007-spawn-transaction.md index 341d5f4..dd057f2 100644 --- a/docs/adr/0007-spawn-transaction.md +++ b/docs/adr/0007-spawn-transaction.md @@ -1,6 +1,6 @@ # 0007 — Process creation is transactional -Status: accepted (2026-08-02) +Status: accepted (2026-08-02); ADR 0016 moves the rollback of the created process into its handle ## Context diff --git a/docs/adr/0013-repository-laws-are-enforced-mechanically.md b/docs/adr/0013-repository-laws-are-enforced-mechanically.md index 3d569a8..a49cd3d 100644 --- a/docs/adr/0013-repository-laws-are-enforced-mechanically.md +++ b/docs/adr/0013-repository-laws-are-enforced-mechanically.md @@ -1,6 +1,6 @@ # 0013 — Repository laws are enforced mechanically -Status: accepted (2026-09-25) +Status: accepted (2026-09-25); ADR 0016 extends the trait-object rule to every `dyn` ## Context diff --git a/docs/adr/0016-spawn-state-is-explicit-and-typed.md b/docs/adr/0016-spawn-state-is-explicit-and-typed.md new file mode 100644 index 0000000..d11634e --- /dev/null +++ b/docs/adr/0016-spawn-state-is-explicit-and-typed.md @@ -0,0 +1,40 @@ +# 0016 — Spawn state is explicit and typed + +Status: accepted (2026-10-13); moves the rollback of ADR 0007 into the process handle, and extends the trait-object rule of ADR 0013 + +## Context + +A suspended child and an uncommitted transaction kept their process in an `Option`, so that a consuming method could move it out past the type's `Drop`. +Each read of it needed an `expect`, and the lint registry excused both. +One function acquired every creation resource, and the registry excused its length too. + +Handles travelled as `BorrowedHandle`, and handle values as `isize`. +Nothing stopped a thread handle from reaching a call for processes, or a value of the creator's table from joining the values an alternate parent's child inherits. +The attribute list held pointers to values that the transaction kept alive by convention. + +PR #14 recorded in its ADR 0009 that trait objects, runtime reflection, shared interior mutability, global state, and hash-ordered collections hide what the compiler needs to check a spawn. +Main dropped #14, and with it those rules. + +## Decision + +- The process handle of a spawn carries its rollback. + Until the main thread runs, dropping the handle terminates the process. + A transaction, a `SuspendedChild`, and a `Child` own their parts directly, and `resume` and the commits move them out without an `Option`. + A running spawn commits by resuming a `SuspendedChild`, so both paths share one resume. +- The creation splits into functions for the standard I/O, the private Job, the Job list, the working directory, and the attribute list. +- Each handle names its object type. + `Owned` and `Borrowed<'a, K>` name a process, a thread, a Job, a pipe end, a standard stream, or a handle the caller supplied, and each wrapper in `sys` takes the type its Win32 call needs. +- Handle values carry the table they belong to. + `Value` names the alternate parent and the Jobs, and `Value` names the inherited handles and the standard handles. +- `AttributeList<'a>` borrows the values its attributes point to, so the compiler keeps them alive and in place until `DeleteProcThreadAttributeList` runs. +- `cargo xtask gates` refuses a trait object in every file, except as the return type of `source` in an implementation of `Error`. + In library code it also refuses `Rc`, `Arc`, `Weak`, cell types, locks, atomics, `static` items, one-time initialization, `thread_local`, `Any`, `TypeId`, type names, and hash-based maps and sets. + Items under `cfg(test)` or `cfg(kani)`, and `src/failure_tests.rs`, keep their thread-local state, counters, and downcasts. +- `deny.toml` bans the crates #14 banned, which erase errors, erase serialization or futures, register items at run time, or keep implicit global state. + +## Consequences + +- The registry holds no `expect_used` or `too_many_lines` exception for `src/child.rs` or `src/transaction.rs`. +- A handle of the wrong kind, a value of the wrong table, or an attribute value that dies before its list fails to compile. +- Concurrency in the library needs a typed boundary and a model before it can pass the gate. +- The output readers of `Child::wait_with_output` stay threads that own what they read, so they share no state. diff --git a/public-api/windows-spawn.txt b/public-api/windows-spawn.txt index 7c86c2d..2b85032 100644 --- a/public-api/windows-spawn.txt +++ b/public-api/windows-spawn.txt @@ -878,8 +878,6 @@ pub fn windows_spawn::SuspendedChild::main_thread_handle(&self) -> std::os::wind pub fn windows_spawn::SuspendedChild::resume(self) -> windows_spawn::Result impl core::fmt::Debug for windows_spawn::SuspendedChild pub fn windows_spawn::SuspendedChild::fmt(&self, &mut core::fmt::Formatter<'_>) -> core::fmt::Result -impl core::ops::drop::Drop for windows_spawn::SuspendedChild -pub fn windows_spawn::SuspendedChild::drop(&mut self) impl std::os::windows::io::handle::AsHandle for windows_spawn::SuspendedChild pub fn windows_spawn::SuspendedChild::as_handle(&self) -> std::os::windows::io::handle::BorrowedHandle<'_> impl std::os::windows::io::raw::AsRawHandle for windows_spawn::SuspendedChild diff --git a/src/child.rs b/src/child.rs index a99c260..f5cbe66 100644 --- a/src/child.rs +++ b/src/child.rs @@ -7,6 +7,7 @@ use std::thread; use crate::error::{Operation, Phase, Result, SpawnError}; use crate::handles::{windows, Job, Stdio}; +use crate::resource::{Borrowed, Owned, Pipe, Process, Thread}; use crate::sys; /// The writable parent end of a child's standard-input pipe. @@ -14,12 +15,12 @@ use crate::sys; /// Like the output ends, it converts into [`Stdio`] and into [`OwnedHandle`]. #[derive(Debug)] pub struct ChildStdin { - handle: OwnedHandle, + handle: Owned, } impl Write for ChildStdin { fn write(&mut self, buffer: &[u8]) -> io::Result { - sys::write_handle(self.handle.as_handle(), buffer) + sys::write_handle(self.handle.borrow(), buffer) } fn flush(&mut self) -> io::Result<()> { @@ -29,7 +30,7 @@ impl Write for ChildStdin { impl Write for &ChildStdin { fn write(&mut self, buffer: &[u8]) -> io::Result { - sys::write_handle(self.handle.as_handle(), buffer) + sys::write_handle(self.handle.borrow(), buffer) } fn flush(&mut self) -> io::Result<()> { @@ -69,12 +70,12 @@ impl AsRawHandle for ChildStdin { /// ``` #[derive(Debug)] pub struct ChildStdout { - handle: OwnedHandle, + handle: Owned, } impl Read for ChildStdout { fn read(&mut self, buffer: &mut [u8]) -> io::Result { - sys::read_handle(self.handle.as_handle(), buffer) + sys::read_handle(self.handle.borrow(), buffer) } } @@ -93,12 +94,12 @@ impl AsRawHandle for ChildStdout { /// The readable parent end of a child's standard-error pipe. #[derive(Debug)] pub struct ChildStderr { - handle: OwnedHandle, + handle: Owned, } impl Read for ChildStderr { fn read(&mut self, buffer: &mut [u8]) -> io::Result { - sys::read_handle(self.handle.as_handle(), buffer) + sys::read_handle(self.handle.borrow(), buffer) } } @@ -116,37 +117,79 @@ impl AsRawHandle for ChildStderr { impl From for OwnedHandle { fn from(stream: ChildStdin) -> Self { - stream.handle + stream.handle.into_inner() } } impl From for OwnedHandle { fn from(stream: ChildStdout) -> Self { - stream.handle + stream.handle.into_inner() } } impl From for OwnedHandle { fn from(stream: ChildStderr) -> Self { - stream.handle + stream.handle.into_inner() } } impl From for Stdio { fn from(stream: ChildStdin) -> Self { - Self::from(stream.handle) + Self::from(OwnedHandle::from(stream)) } } impl From for Stdio { fn from(stream: ChildStdout) -> Self { - Self::from(stream.handle) + Self::from(OwnedHandle::from(stream)) } } impl From for Stdio { fn from(stream: ChildStderr) -> Self { - Self::from(stream.handle) + Self::from(OwnedHandle::from(stream)) + } +} + +/// The crate's handle to a process it created. +/// +/// Until the main thread has run, dropping the handle terminates the process, so a spawn that fails, or a suspended child that is dropped, leaves nothing running. +#[derive(Debug)] +pub(crate) struct ProcessHandle { + handle: Owned, + unstarted: bool, +} + +impl ProcessHandle { + /// Owns a process whose main thread hasn't run. + pub(crate) const fn unstarted(handle: Owned) -> Self { + Self { + handle, + unstarted: true, + } + } + + /// Records that the main thread runs, so dropping the handle leaves the process running. + pub(crate) fn mark_started(&mut self) { + self.unstarted = false; + } + + pub(crate) fn borrow(&self) -> Borrowed<'_, Process> { + self.handle.borrow() + } +} + +impl Drop for ProcessHandle { + fn drop(&mut self) { + if self.unstarted { + drop(sys::terminate_process(self.handle.borrow(), 1)); + } + } +} + +impl AsHandle for ProcessHandle { + fn as_handle(&self) -> BorrowedHandle<'_> { + self.handle.as_handle() } } @@ -157,7 +200,9 @@ impl From for Stdio { /// Dropping the value leaves the process running, unless [`crate::Command::own_process_tree`] made it the owner of the tree. #[derive(Debug)] pub struct Child { - /// Declared first so kill-on-close runs before the pipe and process handles drop. + /// Declared first, so a process that never ran ends with the exit code of a termination before its Job closes. + process: ProcessHandle, + /// Declared before the pipes, so closing the Job ends the tree while the pipes still hold their ends. kill_job: Option, /// A pipe connected to the child's standard input, when requested. pub stdin: Option, @@ -165,26 +210,25 @@ pub struct Child { pub stdout: Option, /// A pipe connected to the child's standard error, when requested. pub stderr: Option, - process: OwnedHandle, pid: u32, exit: Option, } impl Child { pub(crate) fn new( - process: OwnedHandle, + process: ProcessHandle, pid: u32, kill_job: Option, - stdin: Option, - stdout: Option, - stderr: Option, + stdin: Option>, + stdout: Option>, + stderr: Option>, ) -> Self { Self { + process, kill_job, stdin: stdin.map(|handle| ChildStdin { handle }), stdout: stdout.map(|handle| ChildStdout { handle }), stderr: stderr.map(|handle| ChildStderr { handle }), - process, pid, exit: None, } @@ -212,10 +256,10 @@ impl Child { if self.exit.is_some() { return Ok(()); } - let Err(error) = sys::terminate_process(self.process.as_handle(), 1) else { + let Err(error) = sys::terminate_process(self.process.borrow(), 1) else { return Ok(()); }; - if matches!(sys::try_wait_process(self.process.as_handle()), Ok(true)) { + if matches!(sys::try_wait_process(self.process.borrow()), Ok(true)) { return Ok(()); } Err(SpawnError::windows( @@ -238,7 +282,7 @@ impl Child { windows( Phase::Runtime, Operation::WaitProcess, - sys::wait_process(self.process.as_handle()), + sys::wait_process(self.process.borrow()), )?; let status = self.exit_status()?; self.exit = Some(status); @@ -257,7 +301,7 @@ impl Child { let exited = windows( Phase::Runtime, Operation::WaitProcess, - sys::try_wait_process(self.process.as_handle()), + sys::try_wait_process(self.process.borrow()), )?; if !exited { return Ok(None); @@ -271,7 +315,7 @@ impl Child { windows( Phase::Runtime, Operation::QueryExitCode, - sys::exit_status(self.process.as_handle()), + sys::exit_status(self.process.borrow()), ) } @@ -291,14 +335,14 @@ impl Child { let ids = windows( Phase::Runtime, Operation::QueryJob, - sys::job_process_ids(job.as_handle()), + sys::job_process_ids(job.borrow()), )?; let mut members = Vec::with_capacity(ids.len()); for id in ids { members.extend(windows( Phase::Runtime, Operation::OpenProcess, - sys::open_job_process(id, job.as_handle()), + sys::open_job_process(id, job.borrow()), )?); } drop(self.stdin.take()); @@ -309,7 +353,7 @@ impl Child { windows( Phase::Runtime, Operation::WaitProcess, - sys::wait_process(member.as_handle()), + sys::wait_process(member.borrow()), )?; } } @@ -331,11 +375,11 @@ impl Child { let stdout_reader = self .stdout .take() - .map(|stream| thread::spawn(move || drain_output(stream.handle.as_handle()))); + .map(|stream| thread::spawn(move || drain_output(stream.handle.borrow()))); let stderr_reader = self .stderr .take() - .map(|stream| thread::spawn(move || drain_output(stream.handle.as_handle()))); + .map(|stream| thread::spawn(move || drain_output(stream.handle.borrow()))); let status = self.wait(); let termination = self @@ -361,11 +405,11 @@ impl AsHandle for Child { impl AsRawHandle for Child { fn as_raw_handle(&self) -> RawHandle { - self.process.as_raw_handle() + self.process.as_handle().as_raw_handle() } } -fn drain_output(handle: BorrowedHandle<'_>) -> Result> { +fn drain_output(handle: Borrowed<'_, Pipe>) -> Result> { let mut bytes = Vec::new(); let mut buffer = [0_u8; 8_192]; loop { @@ -420,24 +464,19 @@ fn join_readers(stdout: Option, stderr: Option) -> Result<(Vec, - main_thread: OwnedHandle, + child: Child, + main_thread: Owned, } impl SuspendedChild { - pub(crate) const fn new(child: Child, main_thread: OwnedHandle) -> Self { - Self { - pid: child.id(), - child: Some(child), - main_thread, - } + pub(crate) const fn new(child: Child, main_thread: Owned) -> Self { + Self { child, main_thread } } /// Returns the process identifier captured at creation. #[must_use] pub const fn id(&self) -> u32 { - self.pid + self.child.id() } /// Borrows the suspended process's main thread handle. @@ -455,46 +494,33 @@ impl SuspendedChild { /// Returns a Windows error if the main thread can't be resumed, /// and one of kind [`io::ErrorKind::InvalidData`] if another thread suspended it again, so its suspend count was not one. /// The process is terminated in both cases. - pub fn resume(mut self) -> Result { + pub fn resume(self) -> Result { + let Self { + mut child, + main_thread, + } = self; let previous = windows( Phase::Resume, Operation::ResumeThread, - sys::resume_thread(self.main_thread.as_handle()), + sys::resume_thread(main_thread.borrow()), )?; if previous != 1 { return Err(SpawnError::unexpected_suspend_count(previous)); } - self.child.take().ok_or_else(|| { - SpawnError::windows( - Phase::Resume, - Operation::ResumeThread, - io::Error::other("suspended child lost its process"), - ) - }) + child.process.mark_started(); + Ok(child) } } impl AsHandle for SuspendedChild { - #[allow(clippy::expect_used)] fn as_handle(&self) -> BorrowedHandle<'_> { - self.child - .as_ref() - .expect("a suspended child owns its process until resume") - .as_handle() + self.child.as_handle() } } impl AsRawHandle for SuspendedChild { fn as_raw_handle(&self) -> RawHandle { - self.as_handle().as_raw_handle() - } -} - -impl Drop for SuspendedChild { - fn drop(&mut self) { - if let Some(child) = &mut self.child { - drop(child.kill()); - } + self.child.as_raw_handle() } } @@ -566,25 +592,52 @@ mod tests { assert!(alone.wait_tree().unwrap().code().is_some()); let mut owner = command.own_process_tree().spawn().unwrap(); - let job = owner.kill_job.as_ref().unwrap().as_handle(); + let job = owner.kill_job.as_ref().unwrap().borrow(); let ids = sys::job_process_ids(job).unwrap(); assert_eq!(ids, [usize::try_from(owner.id()).unwrap()]); let member = sys::open_job_process(ids[0], job).unwrap().unwrap(); - assert!(!sys::try_wait_process(member.as_handle()).unwrap()); + assert!(!sys::try_wait_process(member.borrow()).unwrap()); let outsider = usize::try_from(std::process::id()).unwrap(); assert!(sys::open_job_process(outsider, job).unwrap().is_none()); assert!(sys::open_job_process(0, job).unwrap().is_none()); assert!(owner.wait_tree().unwrap().code().is_some()); assert_eq!( - sys::job_process_ids(owner.kill_job.as_ref().unwrap().as_handle()).unwrap(), + sys::job_process_ids(owner.kill_job.as_ref().unwrap().borrow()).unwrap(), Vec::::new() ); } + /// A process that survives its handle reads the end of its input and exits 5, and a terminated one exits 1. + #[test] + fn only_a_process_that_never_ran_ends_when_its_handle_drops() { + let mut command = crate::Command::new("cmd.exe"); + command + .args(["/D", "/S", "/C"]) + .raw_arg("set /p x=&exit /b 5") + .stdin(Stdio::piped()) + .stdout(Stdio::null()) + .stderr(Stdio::null()); + let suspended = command.spawn_suspended().unwrap(); + let mut unstarted = sys::test_support::ProcessExitGuard::watch(suspended.as_handle()); + let thread = sys::test_support::duplicate(suspended.main_thread_handle()); + drop(suspended); + sys::test_support::tempt_resume(thread.as_handle()); + assert_eq!(unstarted.exit_code(), 1); + + for child in [ + command.spawn().unwrap(), + command.spawn_suspended().unwrap().resume().unwrap(), + ] { + let mut started = sys::test_support::ProcessExitGuard::watch(child.as_handle()); + drop(child); + assert_eq!(started.exit_code(), 5); + } + } + #[test] fn reader_errors_reach_the_caller() { let (reader, writer) = sys::test_support::pipe(); - assert!(drain_output(writer.as_handle()).is_err()); + assert!(drain_output(Borrowed::adopt(writer.as_handle())).is_err()); drop(reader); let mut exited = crate::Command::new("cmd.exe"); @@ -595,12 +648,16 @@ mod tests { .stderr(Stdio::null()); let source = exited.spawn().unwrap(); let (unreadable_reader, unreadable) = sys::test_support::pipe(); + let mut process = ProcessHandle::unstarted(Owned::adopt(sys::test_support::duplicate( + source.as_handle(), + ))); + process.mark_started(); let child = Child::new( - sys::test_support::duplicate(source.as_handle()), + process, source.id(), None, None, - Some(unreadable), + Some(Owned::adopt(unreadable)), None, ); assert!(child.wait_with_output().is_err()); diff --git a/src/command.rs b/src/command.rs index d5a6743..49c72cf 100644 --- a/src/command.rs +++ b/src/command.rs @@ -2,7 +2,7 @@ use std::ffi::{OsStr, OsString}; use std::fmt; -use std::os::windows::io::{AsHandle, OwnedHandle}; +use std::os::windows::io::AsHandle; use std::path::{Path, PathBuf}; use std::process::{ExitStatus, Output}; @@ -13,6 +13,7 @@ use crate::handles::Stdio; use crate::mitigation::MitigationPolicy; use crate::options::{ConsoleMode, Creation, PriorityClass, SpawnOptions}; use crate::plan::{IoMode, SpawnPlan}; +use crate::resource::{Borrowed, Foreign, Owned}; use crate::sys; use crate::transaction::SpawnTransaction; @@ -20,7 +21,7 @@ use crate::transaction::SpawnTransaction; pub(crate) enum Arg { Text(OsString), Raw(OsString), - Handle(OwnedHandle), + Handle(Owned), } #[derive(Debug)] @@ -32,7 +33,7 @@ pub(crate) enum EnvOp { #[derive(Debug)] pub(crate) enum EnvValue { Text(OsString), - Handle(OwnedHandle), + Handle(Owned), } /// A reusable description of a Windows process launch, with the API of [`std::process::Command`]. @@ -434,8 +435,8 @@ impl Command { } /// A private, non-inheritable duplicate that the command owns until a spawn lowers it. -fn private_duplicate(handle: &T) -> Result { - sys::duplicate_local(handle.as_handle(), false).map_err(|error| { +fn private_duplicate(handle: &T) -> Result> { + sys::duplicate_local(Borrowed::foreign(handle), false).map_err(|error| { SpawnError::windows(Phase::Preparation, Operation::DuplicateLocalHandle, error) }) } diff --git a/src/failure_tests.rs b/src/failure_tests.rs index 2bfd9d5..f125b6a 100644 --- a/src/failure_tests.rs +++ b/src/failure_tests.rs @@ -60,13 +60,15 @@ fn pipe(operation: crate::Operation, outcome: io::Result) -> Result { /// A private duplicate, typed as the crate types its own duplicates. fn duplicate(handle: &impl AsHandle) -> Result { - crate::sys::duplicate_local(handle.as_handle(), false).map_err(|error| { - SpawnError::windows( - Phase::Preparation, - crate::Operation::DuplicateLocalHandle, - error, - ) - }) + crate::sys::duplicate_local(crate::resource::Borrowed::foreign(handle), false) + .map(crate::resource::Owned::into_inner) + .map_err(|error| { + SpawnError::windows( + Phase::Preparation, + crate::Operation::DuplicateLocalHandle, + error, + ) + }) } /// Returns true if `error` holds the injected failure, as the failure itself or as one that a release met. diff --git a/src/handles.rs b/src/handles.rs index a6de165..2180a34 100644 --- a/src/handles.rs +++ b/src/handles.rs @@ -7,6 +7,7 @@ use std::marker::PhantomData; use std::os::windows::io::{AsHandle, AsRawHandle, BorrowedHandle, OwnedHandle, RawHandle}; use crate::error::{Operation, Phase, Result, SpawnError}; +use crate::resource::{Borrowed, JobObject, Owned, Process, Stream}; use crate::sys; /// Runs one Windows operation and types its failure with the phase and operation. @@ -26,7 +27,7 @@ pub(crate) enum StdioInner { Inherit, Null, Piped, - Owned(OwnedHandle), + Owned(Owned), /// The caller's standard output, in whichever slot the child receives it. CallerStdout, /// The caller's standard error, in whichever slot the child receives it. @@ -83,16 +84,16 @@ impl Stdio { windows( Phase::Preparation, Operation::DuplicateLocalHandle, - sys::duplicate_local(source.as_handle(), false), + sys::duplicate_local(Borrowed::foreign(source), false), ) - .map(Self::from) + .map(|duplicate| Self::from(duplicate.into_inner())) } } impl From for Stdio { fn from(handle: OwnedHandle) -> Self { Self { - inner: StdioInner::Owned(handle), + inner: StdioInner::Owned(Owned::adopt(handle)), } } } @@ -124,7 +125,7 @@ impl From for Stdio { /// A process handle validated for use as `PROC_THREAD_ATTRIBUTE_PARENT_PROCESS`. #[derive(Debug)] pub struct ParentProcess { - handle: OwnedHandle, + handle: Owned, } impl ParentProcess { @@ -151,9 +152,15 @@ impl ParentProcess { windows( Phase::Preparation, Operation::OpenProcess, - sys::validate_process_handle(handle.as_handle()), + sys::validate_process_handle(Borrowed::foreign(&handle)), )?; - Ok(Self { handle }) + Ok(Self { + handle: Owned::adopt(handle), + }) + } + + pub(crate) fn borrow(&self) -> Borrowed<'_, Process> { + self.handle.borrow() } } @@ -171,14 +178,14 @@ impl AsRawHandle for ParentProcess { impl From for OwnedHandle { fn from(parent: ParentProcess) -> Self { - parent.handle + parent.handle.into_inner() } } /// An owned Windows Job object. #[derive(Debug)] pub struct Job { - handle: OwnedHandle, + handle: Owned, } impl Job { @@ -201,9 +208,11 @@ impl Job { windows( Phase::Preparation, Operation::QueryJob, - sys::validate_job_handle(handle.as_handle()), + sys::validate_job_handle(Borrowed::foreign(&handle)), )?; - Ok(Self { handle }) + Ok(Self { + handle: Owned::adopt(handle), + }) } /// Creates an independent duplicate of this Job handle. @@ -215,7 +224,7 @@ impl Job { windows( Phase::Preparation, Operation::DuplicateLocalHandle, - sys::duplicate_local(self.handle.as_handle(), false), + sys::duplicate_local(self.handle.borrow(), false), ) .map(|handle| Self { handle }) } @@ -231,7 +240,7 @@ impl Job { windows( Phase::Runtime, Operation::AssignJob, - sys::assign_job(self.handle.as_handle(), process.as_handle()), + sys::assign_job(self.handle.borrow(), Borrowed::foreign(process)), ) } @@ -244,7 +253,7 @@ impl Job { windows( Phase::Runtime, Operation::TerminateJob, - sys::terminate_job(self.handle.as_handle(), exit_code), + sys::terminate_job(self.handle.borrow(), exit_code), ) } @@ -259,9 +268,13 @@ impl Job { windows( Phase::Preparation, Operation::ConfigureJob, - sys::set_job_kill_on_close(self.handle.as_handle(), kill), + sys::set_job_kill_on_close(self.handle.borrow(), kill), ) } + + pub(crate) fn borrow(&self) -> Borrowed<'_, JobObject> { + self.handle.borrow() + } } impl AsHandle for Job { @@ -278,7 +291,7 @@ impl AsRawHandle for Job { impl From for OwnedHandle { fn from(job: Job) -> Self { - job.handle + job.handle.into_inner() } } @@ -330,6 +343,13 @@ mod tests { const ACCESS_DENIED: i32 = 5; + /// A private duplicate of a handle whose kind the crate doesn't know. + fn duplicate(source: &T) -> OwnedHandle { + sys::duplicate_local(Borrowed::foreign(source), false) + .unwrap() + .into_inner() + } + #[test] fn stored_duplicates_are_private() -> Result<()> { use crate::sys::test_support::is_inheritable; @@ -374,25 +394,20 @@ mod tests { let _host_guard = sys::test_support::ProcessExitGuard::watch(host.as_handle()); let parent = ParentProcess::open(host.id()).unwrap(); assert!(format!("{parent:?}").contains("ParentProcess")); - let adopted_parent = - ParentProcess::from_handle(sys::duplicate_local(host.as_handle(), false).unwrap()) - .unwrap(); + let adopted_parent = ParentProcess::from_handle(duplicate(&host)).unwrap(); assert_ne!( adopted_parent.as_handle().as_raw_handle(), std::ptr::null_mut() ); let job = Job::create().unwrap(); - let duplicate = job.duplicate().unwrap(); - let adopted_job = Job::from_handle(duplicate.handle).unwrap(); + let adopted_job = Job::from_handle(OwnedHandle::from(job.duplicate().unwrap())).unwrap(); adopted_job.set_kill_on_close(true).unwrap(); adopted_job.set_kill_on_close(false).unwrap(); let file = File::open("NUL").unwrap(); - let not_process = sys::duplicate_local(file.as_handle(), false).unwrap(); - assert!(ParentProcess::from_handle(not_process).is_err()); - let not_job = sys::duplicate_local(file.as_handle(), false).unwrap(); - assert!(Job::from_handle(not_job).is_err()); + assert!(ParentProcess::from_handle(duplicate(&file)).is_err()); + assert!(Job::from_handle(duplicate(&file)).is_err()); drop(host); } } diff --git a/src/lib.rs b/src/lib.rs index a91e6f1..01556a2 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -26,6 +26,8 @@ mod options; #[cfg(windows)] mod plan; #[cfg(windows)] +mod resource; +#[cfg(windows)] #[allow(unsafe_code)] mod sys; #[cfg(windows)] diff --git a/src/resource.rs b/src/resource.rs new file mode 100644 index 0000000..6e0ce39 --- /dev/null +++ b/src/resource.rs @@ -0,0 +1,220 @@ +//! Handles typed by the object they name, and handle values typed by the table they belong to. +//! +//! A `BorrowedHandle` or an `isize` would let a thread handle reach a call for processes, +//! or a value of the creator's table join a list of the inheritor's values; these types make both a compile error. + +use std::fmt; +use std::marker::PhantomData; +use std::os::windows::io::{AsHandle, AsRawHandle, BorrowedHandle, OwnedHandle, RawHandle}; + +/// A process. +pub(crate) enum Process {} +/// A thread. +pub(crate) enum Thread {} +/// A Job object. +pub(crate) enum JobObject {} +/// One end of an anonymous pipe. +pub(crate) enum Pipe {} +/// A file, device, pipe, or console handle that a child receives as a standard stream. +pub(crate) enum Stream {} +/// A handle the caller supplied, whose kind the crate doesn't know. +pub(crate) enum Foreign {} + +/// An owned handle, in the current process's table, to an object of kind `K`. +pub(crate) struct Owned { + handle: OwnedHandle, + kind: PhantomData, +} + +impl Owned { + /// Adopts `handle` as a handle to an object of kind `K`. + /// + /// The caller has created the object with an API of that kind, or checked the kind with one. + pub(crate) const fn adopt(handle: OwnedHandle) -> Self { + Self { + handle, + kind: PhantomData, + } + } + + /// Borrows the handle with its kind. + pub(crate) fn borrow(&self) -> Borrowed<'_, K> { + Borrowed { + handle: self.handle.as_handle(), + kind: PhantomData, + } + } + + /// Gives up the kind, for a caller that receives a plain handle. + pub(crate) fn into_inner(self) -> OwnedHandle { + self.handle + } +} + +impl AsHandle for Owned { + fn as_handle(&self) -> BorrowedHandle<'_> { + self.handle.as_handle() + } +} + +impl AsRawHandle for Owned { + fn as_raw_handle(&self) -> RawHandle { + self.handle.as_raw_handle() + } +} + +impl fmt::Debug for Owned { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + self.handle.fmt(formatter) + } +} + +/// A borrowed handle, in the current process's table, to an object of kind `K`. +pub(crate) struct Borrowed<'a, K> { + handle: BorrowedHandle<'a>, + kind: PhantomData, +} + +impl<'a> Borrowed<'a, Foreign> { + /// Borrows a handle the caller supplied. + pub(crate) fn foreign(source: &'a T) -> Self { + Self::adopt(source.as_handle()) + } +} + +impl<'a, K> Borrowed<'a, K> { + /// Borrows `handle` as a handle to an object of kind `K`, which the caller has established. + pub(crate) const fn adopt(handle: BorrowedHandle<'a>) -> Self { + Self { + handle, + kind: PhantomData, + } + } + + /// Returns the plain borrowed handle. + pub(crate) const fn untyped(self) -> BorrowedHandle<'a> { + self.handle + } +} + +impl Clone for Borrowed<'_, K> { + fn clone(&self) -> Self { + *self + } +} + +impl Copy for Borrowed<'_, K> {} + +/// The table of the process that creates the child. +pub(crate) enum Creator {} +/// The table of the process the child inherits from: the creator, or the alternate parent. +pub(crate) enum Inheritor {} + +/// A handle value valid in the handle table `T`. +/// +/// Its layout is an `isize`'s, so a slice of values is the array an attribute list reads. +#[repr(transparent)] +pub(crate) struct Value { + raw: isize, + table: PhantomData, +} + +impl Value { + pub(crate) const fn as_raw(self) -> isize { + self.raw + } +} + +impl Value { + /// The value of a handle in the current process's table. + pub(crate) fn of(handle: BorrowedHandle<'_>) -> Self { + Self { + raw: crate::sys::handle_value(handle.as_raw_handle()), + table: PhantomData, + } + } + + /// The same value in the inheritor's table, which is the creator's when no alternate parent is named. + pub(crate) const fn inherited(self) -> Value { + Value { + raw: self.raw, + table: PhantomData, + } + } +} + +impl Value { + /// The value of a standard stream that a child doesn't receive. + pub(crate) const INVALID: Self = Self { + raw: crate::sys::INVALID_RAW_HANDLE, + table: PhantomData, + }; + + /// The value of a duplicate in the alternate parent's table. + pub(crate) const fn in_parent(raw: isize) -> Self { + Self { + raw, + table: PhantomData, + } + } +} + +impl Clone for Value { + fn clone(&self) -> Self { + *self + } +} + +impl Copy for Value {} + +impl PartialEq for Value { + fn eq(&self, other: &Self) -> bool { + self.raw == other.raw + } +} + +impl Eq for Value {} + +impl fmt::Display for Value { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + self.raw.fmt(formatter) + } +} + +impl fmt::Debug for Value { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + self.raw.fmt(formatter) + } +} + +#[cfg(test)] +mod tests { + use std::fs::File; + use std::mem::size_of; + + use super::*; + + #[test] + fn values_keep_the_layout_and_text_of_their_number() { + assert_eq!(size_of::>(), size_of::()); + let file = File::open("NUL").unwrap(); + let value = Value::::of(file.as_handle()); + assert_eq!(value.to_string(), value.as_raw().to_string()); + assert_eq!(format!("{value:?}"), value.as_raw().to_string()); + assert_eq!(value.inherited().as_raw(), value.as_raw()); + assert_eq!(Value::::in_parent(42).as_raw(), 42); + assert_eq!(Value::::INVALID.as_raw(), -1); + let copy = value; + assert_eq!(copy, value); + assert_ne!(Value::::in_parent(1), Value::in_parent(2)); + + let owned = Owned::::adopt(OwnedHandle::from(file)); + assert_eq!( + owned.borrow().untyped().as_raw_handle(), + owned.as_raw_handle() + ); + assert!(format!("{owned:?}").contains("OwnedHandle")); + let raw = owned.as_raw_handle(); + assert_eq!(owned.into_inner().as_raw_handle(), raw); + } +} diff --git a/src/sys.rs b/src/sys.rs index b8349b0..c4a84eb 100644 --- a/src/sys.rs +++ b/src/sys.rs @@ -3,6 +3,7 @@ use std::cmp::Ordering; use std::ffi::{c_void, OsString}; use std::io; +use std::marker::PhantomData; use std::mem::{size_of, size_of_val, ManuallyDrop}; use std::os::windows::ffi::OsStringExt; use std::os::windows::io::{AsRawHandle, BorrowedHandle, FromRawHandle, OwnedHandle}; @@ -49,6 +50,9 @@ use crate::domain::attribute_rules::{self, SizeFault}; use crate::domain::creation_flag_rules::{creation_word, ConsoleFlag, PriorityFlag}; use crate::domain::environment_rules; use crate::handles::BorrowedPseudoConsole; +use crate::resource::{ + Borrowed, Creator, Foreign, Inheritor, JobObject, Owned, Pipe, Process, Stream, Thread, Value, +}; pub(crate) const INVALID_RAW_HANDLE: isize = -1; @@ -94,21 +98,23 @@ pub(crate) enum NullAccess { Write, } -pub(crate) fn duplicate_local( - source: BorrowedHandle<'_>, +/// Duplicates a handle in the current process, keeping its kind. +pub(crate) fn duplicate_local( + source: Borrowed<'_, K>, inheritable: bool, -) -> io::Result { +) -> io::Result> { #[cfg(test)] fault::check(fault::Call::DuplicateLocal)?; // SAFETY: `GetCurrentProcess` cannot fail and returns a pseudo-handle that stays valid and is never closed. let current = unsafe { GetCurrentProcess() }; duplicate_between( current, - raw(source), + raw(source.untyped()), current, inheritable, DUPLICATE_SAME_ACCESS, ) + .map(Owned::adopt) } fn duplicate_between( @@ -137,15 +143,15 @@ fn duplicate_between( owned(duplicate) } -#[derive(Debug)] +/// A duplicate in an alternate parent's table, which this process closes there. pub(crate) struct RemoteHandle<'a> { - process: BorrowedHandle<'a>, + process: Borrowed<'a, Process>, value: HANDLE, } impl RemoteHandle<'_> { - pub(crate) fn value(&self) -> isize { - handle_value(self.value) + pub(crate) fn value(&self) -> Value { + Value::in_parent(handle_value(self.value)) } /// Closes the remote value and reports a failure, which the drop would discard. @@ -160,7 +166,7 @@ impl RemoteHandle<'_> { // SAFETY: `GetCurrentProcess` cannot fail and returns a pseudo-handle that stays valid and is never closed. let current = unsafe { GetCurrentProcess() }; duplicate_between( - raw(self.process), + raw(self.process.untyped()), self.value, current, false, @@ -170,6 +176,15 @@ impl RemoteHandle<'_> { } } +impl std::fmt::Debug for RemoteHandle<'_> { + fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + formatter + .debug_tuple("RemoteHandle") + .field(&handle_value(self.value)) + .finish() + } +} + /// Closes a remote value that no explicit close reached. impl Drop for RemoteHandle<'_> { fn drop(&mut self) { @@ -177,9 +192,9 @@ impl Drop for RemoteHandle<'_> { } } -pub(crate) fn duplicate_remote<'a>( - source: BorrowedHandle<'_>, - target_process: BorrowedHandle<'a>, +pub(crate) fn duplicate_remote<'a, K>( + source: Borrowed<'_, K>, + target_process: Borrowed<'a, Process>, inheritable: bool, ) -> io::Result> { #[cfg(test)] @@ -189,8 +204,8 @@ pub(crate) fn duplicate_remote<'a>( if unsafe { DuplicateHandle( GetCurrentProcess(), - raw(source), - raw(target_process), + raw(source.untyped()), + raw(target_process.untyped()), &mut value, 0, i32::from(inheritable), @@ -206,7 +221,7 @@ pub(crate) fn duplicate_remote<'a>( }) } -pub(crate) fn standard_handle(stream: StandardStream) -> io::Result> { +pub(crate) fn standard_handle(stream: StandardStream) -> io::Result>> { #[cfg(test)] fault::check(fault::Call::StandardHandle)?; let id = match stream { @@ -221,10 +236,10 @@ pub(crate) fn standard_handle(stream: StandardStream) -> io::Result::adopt(borrowed), false).map(Some) } -pub(crate) fn null_handle(access: NullAccess) -> io::Result { +pub(crate) fn null_handle(access: NullAccess) -> io::Result> { #[cfg(test)] fault::check(fault::Call::NullHandle)?; let name = [u16::from(b'N'), u16::from(b'U'), u16::from(b'L'), 0]; @@ -244,7 +259,7 @@ pub(crate) fn null_handle(access: NullAccess) -> io::Result { ptr::null_mut(), ) }; - owned(handle) + owned(handle).map(Owned::adopt) } fn null_share_mode() -> u32 { @@ -253,12 +268,13 @@ fn null_share_mode() -> u32 { .fold(0, std::ops::BitOr::bitor) } -pub(crate) struct Pipe { - pub(crate) parent: OwnedHandle, - pub(crate) child: OwnedHandle, +/// The two ends of an anonymous pipe: the caller's, and the one the child receives. +pub(crate) struct PipeEnds { + pub(crate) parent: Owned, + pub(crate) child: Owned, } -pub(crate) fn create_pipe(parent_reads: bool) -> io::Result { +pub(crate) fn create_pipe(parent_reads: bool) -> io::Result { #[cfg(test)] fault::check(fault::Call::CreatePipe)?; let mut read = ptr::null_mut(); @@ -270,56 +286,59 @@ pub(crate) fn create_pipe(parent_reads: bool) -> io::Result { // SAFETY: CreatePipe succeeded, so both handles are valid and distinct; they are adopted together. let (read, write) = unsafe { ( - OwnedHandle::from_raw_handle(read), - OwnedHandle::from_raw_handle(write), + Owned::adopt(OwnedHandle::from_raw_handle(read)), + Owned::adopt(OwnedHandle::from_raw_handle(write)), ) }; if parent_reads { - Ok(Pipe { + Ok(PipeEnds { parent: read, child: write, }) } else { - Ok(Pipe { + Ok(PipeEnds { parent: write, child: read, }) } } -pub(crate) fn open_parent_process(pid: u32) -> io::Result { +pub(crate) fn open_parent_process(pid: u32) -> io::Result> { #[cfg(test)] fault::check(fault::Call::OpenProcess)?; // SAFETY: OpenProcess has no pointer preconditions. let handle = unsafe { OpenProcess(PROCESS_CREATE_PROCESS | PROCESS_DUP_HANDLE, 0, pid) }; - owned(handle) + owned(handle).map(Owned::adopt) } -pub(crate) fn validate_process_handle(handle: BorrowedHandle<'_>) -> io::Result<()> { +/// Checks with a process API that a caller's handle names a process. +pub(crate) fn validate_process_handle(handle: Borrowed<'_, Foreign>) -> io::Result<()> { #[cfg(test)] fault::check(fault::Call::ValidateProcess)?; // SAFETY: the borrowed handle is valid for the query. - if unsafe { GetProcessId(raw(handle)) } == 0 { + if unsafe { GetProcessId(raw(handle.untyped())) } == 0 { Err(io::Error::last_os_error()) } else { Ok(()) } } -pub(crate) fn create_job() -> io::Result { +pub(crate) fn create_job() -> io::Result> { #[cfg(test)] fault::check(fault::Call::CreateJob)?; // SAFETY: null arguments request an unnamed Job with default security. - owned(unsafe { CreateJobObjectW(ptr::null(), ptr::null()) }) + owned(unsafe { CreateJobObjectW(ptr::null(), ptr::null()) }).map(Owned::adopt) } -pub(crate) fn validate_job_handle(handle: BorrowedHandle<'_>) -> io::Result<()> { - query_job_limits(handle).map(drop) +/// Checks with a Job API that a caller's handle names a Job. +pub(crate) fn validate_job_handle(handle: Borrowed<'_, Foreign>) -> io::Result<()> { + query_job_limits(handle.untyped()).map(drop) } -pub(crate) fn set_job_kill_on_close(handle: BorrowedHandle<'_>, enable: bool) -> io::Result<()> { +pub(crate) fn set_job_kill_on_close(job: Borrowed<'_, JobObject>, enable: bool) -> io::Result<()> { #[cfg(test)] fault::check(fault::Call::SetJob)?; + let handle = job.untyped(); let mut limits = query_job_limits(handle)?; if enable { limits.BasicLimitInformation.LimitFlags |= JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE; @@ -365,18 +384,22 @@ fn query_job_limits( } } -pub(crate) fn assign_job(job: BorrowedHandle<'_>, process: BorrowedHandle<'_>) -> io::Result<()> { +/// Assigns a process the caller supplied, whose kind `AssignProcessToJobObject` checks. +pub(crate) fn assign_job( + job: Borrowed<'_, JobObject>, + process: Borrowed<'_, Foreign>, +) -> io::Result<()> { #[cfg(test)] fault::check(fault::Call::AssignJob)?; // SAFETY: both handles are valid for the call. - bool_result(unsafe { AssignProcessToJobObject(raw(job), raw(process)) }) + bool_result(unsafe { AssignProcessToJobObject(raw(job.untyped()), raw(process.untyped())) }) } -pub(crate) fn terminate_job(job: BorrowedHandle<'_>, exit_code: u32) -> io::Result<()> { +pub(crate) fn terminate_job(job: Borrowed<'_, JobObject>, exit_code: u32) -> io::Result<()> { #[cfg(test)] fault::check(fault::Call::TerminateJob)?; // SAFETY: the Job handle is valid for the call. - bool_result(unsafe { TerminateJobObject(raw(job), exit_code) }) + bool_result(unsafe { TerminateJobObject(raw(job.untyped()), exit_code) }) } /// The `usize` slots of the two-`DWORD` header that `JOBOBJECT_BASIC_PROCESS_ID_LIST` puts before its identifiers. @@ -394,8 +417,8 @@ const _: () = assert!( /// The identifiers of the Job's processes whose exit the Job hasn't recorded. /// -/// A process leaves the list before its handle is signaled, so an empty list means that no process of the Job still runs. -pub(crate) fn job_process_ids(job: BorrowedHandle<'_>) -> io::Result> { +/// The Job can still list a process for a moment after a wait for it returns, but an empty list means that no process of the Job still runs. +pub(crate) fn job_process_ids(job: Borrowed<'_, JobObject>) -> io::Result> { #[cfg(test)] fault::check(fault::Call::ListJob)?; let mut room = PROCESS_ID_LIST_ROOM; @@ -406,7 +429,7 @@ pub(crate) fn job_process_ids(job: BorrowedHandle<'_>) -> io::Result> // SAFETY: `buffer` is writable for `bytes` bytes and aligned for JOBOBJECT_BASIC_PROCESS_ID_LIST, and the returned-size pointer is null. let listed = unsafe { QueryInformationJobObject( - raw(job), + raw(job.untyped()), JobObjectBasicProcessIdList, buffer.as_mut_ptr().cast(), bytes, @@ -441,8 +464,8 @@ pub(crate) fn job_process_ids(job: BorrowedHandle<'_>) -> io::Result> /// Opens a process of the Job to wait for it, or returns `None` if it has exited or its identifier now names a process outside the Job. pub(crate) fn open_job_process( id: usize, - job: BorrowedHandle<'_>, -) -> io::Result> { + job: Borrowed<'_, JobObject>, +) -> io::Result>> { #[cfg(test)] fault::check(fault::Call::OpenJobProcess)?; let id = @@ -462,12 +485,18 @@ pub(crate) fn open_job_process( }; let mut member = 0; // SAFETY: both handles are valid for the query and `member` is writable. - bool_result(unsafe { IsProcessInJob(process.as_raw_handle(), raw(job), &mut member) })?; - Ok((member != 0).then_some(process)) + bool_result(unsafe { + IsProcessInJob(process.as_raw_handle(), raw(job.untyped()), &mut member) + })?; + Ok((member != 0).then_some(Owned::adopt(process))) } -pub(crate) struct AttributeList { +/// A `PROC_THREAD_ATTRIBUTE_LIST` whose attributes point into values that live for `'a`. +/// +/// Windows reads those values until `DeleteProcThreadAttributeList` runs, so the borrows keep them alive and in place for as long as the list exists. +pub(crate) struct AttributeList<'a> { storage: Box<[usize]>, + values: PhantomData<&'a ()>, } /// Zeroed storage of `words` words, or `OutOfMemory` when no allocation holds them. @@ -482,7 +511,7 @@ fn zeroed_words(words: usize) -> io::Result> { Ok(storage.into_boxed_slice()) } -impl AttributeList { +impl<'a> AttributeList<'a> { pub(crate) fn new(count: u32) -> io::Result { #[cfg(test)] fault::check(fault::Call::AttributeList)?; @@ -510,10 +539,14 @@ impl AttributeList { if unsafe { InitializeProcThreadAttributeList(pointer, count, 0, &mut actual) } == 0 { return Err(io::Error::last_os_error()); } - Ok(Self { storage }) + Ok(Self { + storage, + values: PhantomData, + }) } - pub(crate) fn set_handle_list(&mut self, handles: &[isize]) -> io::Result<()> { + /// Lists the handles the child inherits, by their values in the inheritor's table. + pub(crate) fn set_handle_list(&mut self, handles: &'a [Value]) -> io::Result<()> { self.update( PROC_THREAD_ATTRIBUTE_HANDLE_LIST, handles.as_ptr().cast(), @@ -521,15 +554,16 @@ impl AttributeList { ) } - pub(crate) fn set_parent(&mut self, parent: &isize) -> io::Result<()> { + /// Names the alternate parent by the value of its handle in the creator's table. + pub(crate) fn set_parent(&mut self, parent: &'a Value) -> io::Result<()> { self.update( PROC_THREAD_ATTRIBUTE_PARENT_PROCESS, ptr::addr_of!(*parent).cast(), - size_of::(), + size_of::>(), ) } - pub(crate) fn set_mitigation(&mut self, words: &[u64; 2]) -> io::Result<()> { + pub(crate) fn set_mitigation(&mut self, words: &'a [u64; 2]) -> io::Result<()> { self.update( PROC_THREAD_ATTRIBUTE_MITIGATION_POLICY, words.as_ptr().cast(), @@ -537,7 +571,8 @@ impl AttributeList { ) } - pub(crate) fn set_jobs(&mut self, jobs: &[isize]) -> io::Result<()> { + /// Names the Jobs by the values of their handles in the creator's table. + pub(crate) fn set_jobs(&mut self, jobs: &'a [Value]) -> io::Result<()> { self.update( PROC_THREAD_ATTRIBUTE_JOB_LIST, jobs.as_ptr().cast(), @@ -548,7 +583,7 @@ impl AttributeList { /// Unlike the other attributes, `lpValue` is the `HPCON` value itself, not its address. pub(crate) fn set_pseudoconsole( &mut self, - pseudoconsole: BorrowedPseudoConsole<'_>, + pseudoconsole: BorrowedPseudoConsole<'a>, ) -> io::Result<()> { self.update( PROC_THREAD_ATTRIBUTE_PSEUDOCONSOLE, @@ -561,7 +596,7 @@ impl AttributeList { #[cfg(test)] fault::check(fault::Call::UpdateAttribute)?; let attribute = widen(attribute); - // SAFETY: the list is initialized, `value` points to `bytes` readable bytes or is the `HPCON` value, and the transaction keeps every backing allocation stable until CreateProcessW returns. + // SAFETY: the list is initialized, and `value` is the `HPCON` value or points to `bytes` readable bytes that the setter borrowed for `'a`, so they stay alive and in place until the drop deletes the list. if unsafe { UpdateProcThreadAttribute( self.pointer(), @@ -585,7 +620,7 @@ impl AttributeList { } } -impl Drop for AttributeList { +impl Drop for AttributeList<'_> { fn drop(&mut self) { #[cfg(test)] ATTRIBUTE_LIST_DROPS.fetch_add(1, std::sync::atomic::Ordering::Relaxed); @@ -606,16 +641,17 @@ impl BorrowedPseudoConsole<'_> { pub const unsafe fn borrow_raw(raw: isize) -> Self { Self { raw, - owner: std::marker::PhantomData, + owner: PhantomData, } } } +/// The standard handles of a child, by their values in the inheritor's table. #[derive(Clone, Copy)] pub(crate) struct StandardHandles { - pub(crate) stdin: isize, - pub(crate) stdout: isize, - pub(crate) stderr: isize, + pub(crate) stdin: Value, + pub(crate) stdout: Value, + pub(crate) stderr: Value, } #[derive(Clone, Copy)] @@ -635,12 +671,12 @@ pub(crate) struct ProcessRequest<'a> { pub(crate) policy: u32, pub(crate) console: ConsoleFlag, pub(crate) priority: PriorityFlag, - pub(crate) attributes: Option<&'a AttributeList>, + pub(crate) attributes: Option<&'a AttributeList<'a>>, } pub(crate) struct CreatedProcess { - pub(crate) process: OwnedHandle, - pub(crate) thread: OwnedHandle, + pub(crate) process: Owned, + pub(crate) thread: Owned, pub(crate) pid: u32, } @@ -690,8 +726,8 @@ pub(crate) fn create_process(request: &mut ProcessRequest<'_>) -> io::Result) -> io::Result) -> io::Result<()> { +pub(crate) fn wait_process(process: Borrowed<'_, Process>) -> io::Result<()> { #[cfg(test)] fault::check(fault::Call::WaitProcess)?; // SAFETY: the process handle is valid for the wait. - match unsafe { WaitForSingleObject(raw(process), INFINITE) } { + match unsafe { WaitForSingleObject(raw(process.untyped()), INFINITE) } { WAIT_OBJECT_0 => Ok(()), _ => Err(io::Error::last_os_error()), } } -pub(crate) fn try_wait_process(process: BorrowedHandle<'_>) -> io::Result { +pub(crate) fn try_wait_process(process: Borrowed<'_, Process>) -> io::Result { #[cfg(test)] fault::check(fault::Call::TryWaitProcess)?; // SAFETY: the process handle is valid for the query. - match unsafe { WaitForSingleObject(raw(process), 0) } { + match unsafe { WaitForSingleObject(raw(process.untyped()), 0) } { WAIT_OBJECT_0 => Ok(true), WAIT_TIMEOUT => Ok(false), _ => Err(io::Error::last_os_error()), @@ -1112,7 +1148,7 @@ pub(crate) mod test_support { } } -pub(crate) fn exit_status(process: BorrowedHandle<'_>) -> io::Result { +pub(crate) fn exit_status(process: Borrowed<'_, Process>) -> io::Result { use std::os::windows::process::ExitStatusExt; #[cfg(test)] @@ -1120,25 +1156,25 @@ pub(crate) fn exit_status(process: BorrowedHandle<'_>) -> io::Result let mut code = 0_u32; // SAFETY: `code` is writable and the process handle is valid. - if unsafe { GetExitCodeProcess(raw(process), &mut code) } == 0 { + if unsafe { GetExitCodeProcess(raw(process.untyped()), &mut code) } == 0 { Err(io::Error::last_os_error()) } else { Ok(ExitStatus::from_raw(code)) } } -pub(crate) fn terminate_process(process: BorrowedHandle<'_>, exit_code: u32) -> io::Result<()> { +pub(crate) fn terminate_process(process: Borrowed<'_, Process>, exit_code: u32) -> io::Result<()> { #[cfg(test)] fault::check(fault::Call::TerminateProcess)?; // SAFETY: the process handle is valid for the call. - bool_result(unsafe { TerminateProcess(raw(process), exit_code) }) + bool_result(unsafe { TerminateProcess(raw(process.untyped()), exit_code) }) } -pub(crate) fn resume_thread(thread: BorrowedHandle<'_>) -> io::Result { +pub(crate) fn resume_thread(thread: Borrowed<'_, Thread>) -> io::Result { #[cfg(test)] fault::check(fault::Call::ResumeThread)?; // SAFETY: the thread handle is valid for the call. - let previous = unsafe { ResumeThread(raw(thread)) }; + let previous = unsafe { ResumeThread(raw(thread.untyped())) }; if previous == u32::MAX { Err(io::Error::last_os_error()) } else { @@ -1146,7 +1182,7 @@ pub(crate) fn resume_thread(thread: BorrowedHandle<'_>) -> io::Result { } } -pub(crate) fn read_handle(handle: BorrowedHandle<'_>, buffer: &mut [u8]) -> io::Result { +pub(crate) fn read_handle(handle: Borrowed<'_, Pipe>, buffer: &mut [u8]) -> io::Result { #[cfg(test)] fault::check(fault::Call::ReadHandle)?; if buffer.is_empty() { @@ -1157,7 +1193,7 @@ pub(crate) fn read_handle(handle: BorrowedHandle<'_>, buffer: &mut [u8]) -> io:: // SAFETY: `buffer` is writable for `length` bytes, the handle is valid, and a null OVERLAPPED requests synchronous I/O. if unsafe { ReadFile( - raw(handle), + raw(handle.untyped()), buffer.as_mut_ptr(), length, &mut read, @@ -1176,7 +1212,7 @@ pub(crate) fn read_handle(handle: BorrowedHandle<'_>, buffer: &mut [u8]) -> io:: } } -pub(crate) fn write_handle(handle: BorrowedHandle<'_>, buffer: &[u8]) -> io::Result { +pub(crate) fn write_handle(handle: Borrowed<'_, Pipe>, buffer: &[u8]) -> io::Result { #[cfg(test)] fault::check(fault::Call::WriteHandle)?; if buffer.is_empty() { @@ -1187,7 +1223,7 @@ pub(crate) fn write_handle(handle: BorrowedHandle<'_>, buffer: &[u8]) -> io::Res // SAFETY: `buffer` is readable for `length` bytes, the handle is valid, and a null OVERLAPPED requests synchronous I/O. if unsafe { WriteFile( - raw(handle), + raw(handle.untyped()), buffer.as_ptr(), length, &mut written, @@ -1362,6 +1398,7 @@ fn bool_result(result: i32) -> io::Result<()> { #[cfg(test)] mod tests { use std::fs::File; + use std::io::{Read, Write}; use std::os::windows::ffi::OsStrExt; use std::os::windows::io::{AsHandle, AsRawHandle}; use std::path::PathBuf; @@ -1381,23 +1418,27 @@ mod tests { ); let readable_null = null_handle(NullAccess::Read)?; let writable_null = null_handle(NullAccess::Write)?; - assert_eq!(read_handle(readable_null.as_handle(), &mut [])?, 0); - assert_eq!(write_handle(writable_null.as_handle(), &[])?, 0); - assert_eq!(write_handle(writable_null.as_handle(), b"discard")?, 7); + let mut unread = [0_u8; 1]; + assert_eq!(File::from(readable_null.into_inner()).read(&mut unread)?, 0); + let mut discard = File::from(duplicate_local(writable_null.borrow(), false)?.into_inner()); + assert_eq!(discard.write(b"discard")?, 7); + drop(discard); let parent_reads = create_pipe(true)?; - assert_eq!(write_handle(parent_reads.child.as_handle(), b"a")?, 1); + assert_eq!(read_handle(parent_reads.parent.borrow(), &mut [])?, 0); + assert_eq!(write_handle(parent_reads.child.borrow(), &[])?, 0); + assert_eq!(write_handle(parent_reads.child.borrow(), b"a")?, 1); let mut byte = [0_u8; 1]; - assert_eq!(read_handle(parent_reads.parent.as_handle(), &mut byte)?, 1); + assert_eq!(read_handle(parent_reads.parent.borrow(), &mut byte)?, 1); assert_eq!(byte, [b'a']); let parent_writes = create_pipe(false)?; - assert_eq!(write_handle(parent_writes.parent.as_handle(), b"b")?, 1); - assert_eq!(read_handle(parent_writes.child.as_handle(), &mut byte)?, 1); + assert_eq!(write_handle(parent_writes.parent.borrow(), b"b")?, 1); + assert_eq!(read_handle(parent_writes.child.borrow(), &mut byte)?, 1); assert_eq!(byte, [b'b']); - let private = duplicate_local(writable_null.as_handle(), false)?; - let inheritable = duplicate_local(writable_null.as_handle(), true)?; + let private = duplicate_local(writable_null.borrow(), false)?; + let inheritable = duplicate_local(writable_null.borrow(), true)?; drop((private, inheritable)); let mut host_command = crate::Command::new("cmd.exe"); @@ -1412,8 +1453,12 @@ mod tests { drop(target); let before = process_handle_count(host.as_handle())?; let local_before = process_handle_count(current_process())?; - let remote = duplicate_remote(writable_null.as_handle(), host.as_handle(), true)?; - assert_ne!(remote.value(), 0); + let remote = duplicate_remote( + writable_null.borrow(), + Borrowed::adopt(host.as_handle()), + true, + )?; + assert_ne!(remote.value().as_raw(), 0); assert!(process_handle_count(host.as_handle())? > before); drop(remote); assert_eq!(process_handle_count(host.as_handle())?, before); @@ -1426,8 +1471,8 @@ mod tests { fn jobs_attributes_and_standard_handles_cover_all_ffi_shapes() -> io::Result<()> { let drops_before = ATTRIBUTE_LIST_DROPS.load(std::sync::atomic::Ordering::Relaxed); let job = create_job()?; - validate_job_handle(job.as_handle())?; - set_job_kill_on_close(job.as_handle(), true)?; + validate_job_handle(Borrowed::foreign(&job))?; + set_job_kill_on_close(job.borrow(), true)?; assert_ne!( query_job_limits(job.as_handle())? .BasicLimitInformation @@ -1435,7 +1480,7 @@ mod tests { & JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE, 0 ); - set_job_kill_on_close(job.as_handle(), false)?; + set_job_kill_on_close(job.borrow(), false)?; assert_eq!( query_job_limits(job.as_handle())? .BasicLimitInformation @@ -1444,10 +1489,10 @@ mod tests { 0 ); let process = open_parent_process(std::process::id())?; - let inherited = duplicate_local(job.as_handle(), true)?; - let handles = [handle_value(inherited.as_raw_handle())]; - let jobs = [handle_value(job.as_raw_handle())]; - let parent = handle_value(process.as_raw_handle()); + let inherited = duplicate_local(job.borrow(), true)?; + let handles = [Value::::of(inherited.as_handle()).inherited()]; + let jobs = [Value::::of(job.as_handle())]; + let parent = Value::::of(process.as_handle()); let words = [1_u64, 0_u64]; let mut attributes = AttributeList::new(4)?; attributes.set_handle_list(&handles)?; @@ -1484,9 +1529,9 @@ mod tests { set_standard_handles( &mut ordinary, StartupStdio::Ordinary(StandardHandles { - stdin: 1, - stdout: 2, - stderr: 3, + stdin: Value::in_parent(1), + stdout: Value::in_parent(2), + stderr: Value::in_parent(3), }), ); assert_ne!(ordinary.StartupInfo.dwFlags & STARTF_USESTDHANDLES, 0); @@ -1544,8 +1589,8 @@ mod tests { assert!(is_valid_handle(file.as_raw_handle())); assert!(!is_valid_handle(ptr::null_mut())); assert!(!is_valid_handle(INVALID_HANDLE_VALUE)); - assert!(validate_process_handle(file.as_handle()).is_err()); - assert!(validate_job_handle(file.as_handle()).is_err()); + assert!(validate_process_handle(Borrowed::foreign(&file)).is_err()); + assert!(validate_job_handle(Borrowed::foreign(&file)).is_err()); Ok(()) } @@ -1562,30 +1607,30 @@ mod tests { current_process(), PROCESS_QUERY_LIMITED_INFORMATION, ); - assert!(wait_process(unwaitable.as_handle()).is_err()); - assert!(try_wait_process(unwaitable.as_handle()).is_err()); + assert!(wait_process(Borrowed::adopt(unwaitable.as_handle())).is_err()); + assert!(try_wait_process(Borrowed::adopt(unwaitable.as_handle())).is_err()); let unqueryable = test_support::duplicate_with_access(current_process(), PROCESS_SYNCHRONIZE); - assert!(exit_status(unqueryable.as_handle()).is_err()); + assert!(exit_status(Borrowed::adopt(unqueryable.as_handle())).is_err()); // SAFETY: `GetCurrentThread` cannot fail and returns a pseudo-handle valid for this call. let thread = unsafe { BorrowedHandle::borrow_raw(GetCurrentThread()) }; let unresumable = test_support::duplicate_with_access(thread, PROCESS_SYNCHRONIZE); - assert!(resume_thread(unresumable.as_handle()).is_err()); + assert!(resume_thread(Borrowed::adopt(unresumable.as_handle())).is_err()); let (reader, writer) = test_support::pipe(); - assert!(read_handle(writer.as_handle(), &mut [0_u8; 1]).is_err()); - assert!(write_handle(reader.as_handle(), b"x").is_err()); + assert!(read_handle(Borrowed::adopt(writer.as_handle()), &mut [0_u8; 1]).is_err()); + assert!(write_handle(Borrowed::adopt(reader.as_handle()), b"x").is_err()); let job = create_job()?; let query_only = test_support::duplicate_with_access(job.as_handle(), JOB_OBJECT_QUERY); - assert!(set_job_kill_on_close(query_only.as_handle(), true).is_err()); + assert!(set_job_kill_on_close(Borrowed::adopt(query_only.as_handle()), true).is_err()); assert!(AttributeList::new(u32::MAX).is_err()); + let jobs = [Value::::of(job.as_handle())]; + let words = [1_u64, 0_u64]; let mut full = AttributeList::new(1)?; - let jobs = [handle_value(job.as_raw_handle())]; full.set_jobs(&jobs)?; - let words = [1_u64, 0_u64]; assert!(full.set_mitigation(&words).is_err()); assert_eq!( diff --git a/src/transaction.rs b/src/transaction.rs index c0da846..e2c4c62 100644 --- a/src/transaction.rs +++ b/src/transaction.rs @@ -7,28 +7,32 @@ use std::ffi::{OsStr, OsString}; use std::io; use std::marker::PhantomData; use std::os::windows::ffi::{OsStrExt, OsStringExt}; -use std::os::windows::io::{AsHandle, AsRawHandle, BorrowedHandle, OwnedHandle}; +#[cfg(test)] +use std::os::windows::io::BorrowedHandle; +use std::os::windows::io::{AsHandle, OwnedHandle}; use std::path::{Path, PathBuf}; -use crate::child::{Child, SuspendedChild}; +use crate::child::{Child, ProcessHandle, SuspendedChild}; use crate::command::{Arg, Command, EnvOp, EnvValue}; use crate::domain::attribute_rules; use crate::domain::command_line_rules::{append_argument, append_program, NUL, QUOTE, SPACE}; use crate::domain::environment_rules; use crate::error::{InputField, Operation, Phase, Releases, Result, SpawnError, ValidationError}; -use crate::handles::{windows, Job, Stdio, StdioInner}; +use crate::handles::{windows, BorrowedPseudoConsole, Job, ParentProcess, Stdio, StdioInner}; use crate::plan::{validate_command, Running, Source, SpawnPlan, StandardIo, Streams, Suspended}; +use crate::resource::{Borrowed, Creator, Inheritor, Owned, Pipe, Process, Thread, Value}; use crate::sys::{self, NullAccess, StandardStream}; /// The UTF-16 units `CreateProcessW` accepts in `lpCommandLine`, its terminator included. const COMMAND_LINE_CAPACITY: usize = 32_767; /// Owns the suspended process between its creation and an explicit commit. -/// Dropping before commit terminates the process, which never ran. +/// +/// The child's process handle terminates the process when it drops before a commit resumes the main thread, +/// so dropping an uncommitted transaction ends a process that never ran. pub(crate) struct SpawnTransaction { - created: Option, - kill_job: Option, - stdio: Streams>, + child: Child, + main_thread: Owned, state: PhantomData, } @@ -36,11 +40,7 @@ impl SpawnTransaction { /// Borrows the created process's main thread. #[cfg(test)] pub(crate) fn main_thread(&self) -> BorrowedHandle<'_> { - self.created - .as_ref() - .expect("an uncommitted transaction owns its process") - .thread - .as_handle() + self.main_thread.as_handle() } } @@ -51,9 +51,8 @@ impl SpawnTransaction { /// /// Returns the first failure, together with the failures of the releases that followed it. /// A release that fails after the creation terminates the process, which never ran. - pub(crate) fn new<'options>(plan: &SpawnPlan<'_, 'options, M>) -> Result { - let parent: Option> = plan.options.parent.map(AsHandle::as_handle); - let mut transfer = HandleTransfer::new(parent); + pub(crate) fn new(plan: &SpawnPlan<'_, '_, M>) -> Result { + let mut transfer = HandleTransfer::new(plan.options.parent.map(ParentProcess::borrow)); let created = Self::create(plan, &mut transfer); let mut releases = Releases::default(); transfer.reclaim(&mut releases); @@ -63,156 +62,61 @@ impl SpawnTransaction { } } - #[allow(clippy::too_many_lines)] - fn create<'options>( - plan: &SpawnPlan<'_, 'options, M>, - transfer: &mut HandleTransfer<'options>, - ) -> Result { - let (stdio_values, stdio) = match &plan.stdio { - StandardIo::Ordinary(specs) => { - let prepared = prepare_standard_handles(specs, transfer)?; - let values = sys::StartupStdio::Ordinary(sys::StandardHandles { - stdin: prepared.stdin.child, - stdout: prepared.stdout.child, - stderr: prepared.stderr.child, - }); - let owners = Streams { - stdin: prepared.stdin.parent, - stdout: prepared.stdout.parent, - stderr: prepared.stderr.parent, - }; - (values, owners) - } - StandardIo::PseudoConsole => ( - sys::StartupStdio::PseudoConsole, - Streams { - stdin: None, - stdout: None, - stderr: None, - }, - ), - }; + fn create(plan: &SpawnPlan<'_, '_, M>, transfer: &mut HandleTransfer<'_>) -> Result { + let (stdio, pipes) = prepare_standard_io(&plan.stdio, transfer)?; let creation = &plan.command.creation; - let kill_job = if creation.owns_tree { - let job = Job::create()?; - job.set_kill_on_close(true)?; - Some(job) - } else { - None - }; - let mut job_values: Vec = plan - .options - .jobs - .iter() - .map(|job| sys::handle_value(job.as_handle().as_raw_handle())) - .collect(); - if let Some(job) = &kill_job { - job_values.push(sys::handle_value(job.as_handle().as_raw_handle())); - } - - let command_line = build_command_line(plan.command, transfer)?; + let kill_job = tree_job(creation.owns_tree)?; + let jobs = job_values(&plan.options.jobs, kill_job.as_ref()); + let mut command_line = build_command_line(plan.command, transfer)?; let environment = build_environment(plan.command, transfer)?; - let child_path = environment.path.as_deref(); let application = windows( Phase::Preparation, Operation::ResolveExecutable, - resolve_executable(&plan.command.program, child_path), + resolve_executable(&plan.command.program, environment.path.as_deref()), )?; - let current_dir = plan - .command - .cwd - .as_ref() - .map(|path| wide_nul(path.as_os_str())) - .transpose() - .map_err(|_nul| { - SpawnError::from(ValidationError::InteriorNul { - field: InputField::CurrentDirectory, - }) - })?; - - let inherited_values = transfer.inherited_values().to_vec().into_boxed_slice(); - let parent_value = transfer - .parent() - .map(|handle| Box::new(sys::handle_value(handle.as_raw_handle()))); - let mitigation_words = creation.mitigation.words(); - let mitigation_value = (mitigation_words != [0, 0]).then(|| Box::new(mitigation_words)); - let job_values = job_values.into_boxed_slice(); - let pseudoconsole = plan.options.pseudoconsole; - - let attribute_count = attribute_rules::count(&[ - !inherited_values.is_empty(), - parent_value.is_some(), - mitigation_value.is_some(), - !job_values.is_empty(), - pseudoconsole.is_some(), - ]); - let mut attributes = if attribute_count == 0 { - None - } else { - Some(configure(sys::AttributeList::new(attribute_count))?) - }; - if let Some(list) = &mut attributes { - if !inherited_values.is_empty() { - configure(list.set_handle_list(&inherited_values))?; - } - if let Some(value) = &parent_value { - configure(list.set_parent(value))?; - } - if let Some(value) = &mitigation_value { - configure(list.set_mitigation(value))?; - } - if !job_values.is_empty() { - configure(list.set_jobs(&job_values))?; - } - if let Some(value) = pseudoconsole { - configure(list.set_pseudoconsole(value))?; - } - } + let current_dir = current_directory(plan.command)?; - let mut command_line = command_line; - let mut request = sys::ProcessRequest { - application: &application, - command_line: &mut command_line, - environment: environment.block.as_deref(), - current_dir: current_dir.as_deref(), - stdio: stdio_values, - inherit_handles: !inherited_values.is_empty(), - policy: creation.policy(), - console: creation.console_flag(), - priority: creation.priority_flag(), - attributes: attributes.as_ref(), - }; + let parent = transfer.parent_value(); + let mitigation = creation.mitigation.words(); + let inherited = transfer.inherited_values(); + let attributes = attribute_list(&AttributeValues { + inherited, + parent: parent.as_ref(), + mitigation: (mitigation != [0, 0]).then_some(&mitigation), + jobs: &jobs, + pseudoconsole: plan.options.pseudoconsole, + })?; let created = windows( Phase::Creation, Operation::CreateProcess, - sys::create_process(&mut request), + sys::create_process(&mut sys::ProcessRequest { + application: &application, + command_line: &mut command_line, + environment: environment.block.as_deref(), + current_dir: current_dir.as_deref(), + stdio, + inherit_handles: !inherited.is_empty(), + policy: creation.policy(), + console: creation.console_flag(), + priority: creation.priority_flag(), + attributes: attributes.as_ref(), + }), )?; drop(attributes); Ok(Self { - created: Some(created), - kill_job, - stdio, + child: Child::new( + ProcessHandle::unstarted(created.process), + created.pid, + kill_job, + pipes.stdin, + pipes.stdout, + pipes.stderr, + ), + main_thread: created.thread, state: PhantomData, }) } - - #[allow(clippy::expect_used)] - fn commit_parts(mut self) -> (Child, OwnedHandle) { - let created = self - .created - .take() - .expect("an uncommitted transaction owns its process"); - let child = Child::new( - created.process, - created.pid, - self.kill_job.take(), - self.stdio.stdin.take(), - self.stdio.stdout.take(), - self.stdio.stderr.take(), - ); - (child, created.thread) - } } impl SpawnTransaction { @@ -222,41 +126,125 @@ impl SpawnTransaction { /// /// Returns a failed resume, or a suspend count other than one; the process is terminated then. pub(crate) fn commit_child(self) -> Result { - if let Some(created) = &self.created { - let previous = windows( - Phase::Resume, - Operation::ResumeThread, - sys::resume_thread(created.thread.as_handle()), - )?; - if previous != 1 { - return Err(SpawnError::unexpected_suspend_count(previous)); - } - } - let (child, thread) = self.commit_parts(); - drop(thread); - Ok(child) + SuspendedChild::new(self.child, self.main_thread).resume() } } impl SpawnTransaction { pub(crate) fn commit_suspended(self) -> SuspendedChild { - let (child, thread) = self.commit_parts(); - SuspendedChild::new(child, thread) + SuspendedChild::new(self.child, self.main_thread) + } +} + +/// The private Job of a child that owns its tree, which ends the tree when its last handle closes. +fn tree_job(owns_tree: bool) -> Result> { + if !owns_tree { + return Ok(None); + } + let job = Job::create()?; + job.set_kill_on_close(true)?; + Ok(Some(job)) +} + +/// The values, in the creator's table, of the Jobs the child joins: the caller's, then its private one. +fn job_values(jobs: &[&Job], kill_job: Option<&Job>) -> Vec> { + jobs.iter() + .copied() + .chain(kill_job) + .map(|job| Value::::of(job.as_handle())) + .collect() +} + +/// The child's working directory as a terminated wide string, when the command sets one. +fn current_directory(command: &Command) -> Result>> { + command + .cwd + .as_ref() + .map(|path| wide_nul(path.as_os_str())) + .transpose() + .map_err(|_nul| { + SpawnError::from(ValidationError::InteriorNul { + field: InputField::CurrentDirectory, + }) + }) +} + +/// What the attributes of a spawn point to. +/// +/// The attribute list borrows each value, so every one outlives the list. +struct AttributeValues<'a> { + inherited: &'a [Value], + parent: Option<&'a Value>, + mitigation: Option<&'a [u64; 2]>, + jobs: &'a [Value], + pseudoconsole: Option>, +} + +/// Builds the attribute list of `values`, or returns `None` when the spawn sets no attribute. +fn attribute_list<'a>(values: &AttributeValues<'a>) -> Result>> { + let count = attribute_rules::count(&[ + !values.inherited.is_empty(), + values.parent.is_some(), + values.mitigation.is_some(), + !values.jobs.is_empty(), + values.pseudoconsole.is_some(), + ]); + if count == 0 { + return Ok(None); + } + let mut list = configure(sys::AttributeList::new(count))?; + if !values.inherited.is_empty() { + configure(list.set_handle_list(values.inherited))?; } + if let Some(parent) = values.parent { + configure(list.set_parent(parent))?; + } + if let Some(words) = values.mitigation { + configure(list.set_mitigation(words))?; + } + if !values.jobs.is_empty() { + configure(list.set_jobs(values.jobs))?; + } + if let Some(pseudoconsole) = values.pseudoconsole { + configure(list.set_pseudoconsole(pseudoconsole))?; + } + Ok(Some(list)) } -impl Drop for SpawnTransaction { - fn drop(&mut self) { - if let Some(created) = &self.created { - drop(sys::terminate_process(created.process.as_handle(), 1)); +/// Lowers the child's standard handles, and returns them with the caller's ends of the child's pipes. +fn prepare_standard_io( + stdio: &StandardIo<&Stdio>, + transfer: &mut HandleTransfer<'_>, +) -> Result<(sys::StartupStdio, Streams>>)> { + match stdio { + StandardIo::Ordinary(specs) => { + let prepared = prepare_standard_handles(specs, transfer)?; + let values = sys::StartupStdio::Ordinary(sys::StandardHandles { + stdin: prepared.stdin.child, + stdout: prepared.stdout.child, + stderr: prepared.stderr.child, + }); + let pipes = Streams { + stdin: prepared.stdin.parent, + stdout: prepared.stdout.parent, + stderr: prepared.stderr.parent, + }; + Ok((values, pipes)) } - drop(self.kill_job.take()); + StandardIo::PseudoConsole => Ok(( + sys::StartupStdio::PseudoConsole, + Streams { + stdin: None, + stdout: None, + stderr: None, + }, + )), } } struct PreparedStdio { - child: isize, - parent: Option, + child: Value, + parent: Option>, } fn prepare_standard_handles( @@ -286,7 +274,7 @@ fn prepare_stdio( StdioInner::Null => prepare_null(stream, transfer), StdioInner::Piped => prepare_pipe(stream, transfer), StdioInner::Owned(handle) => Ok(PreparedStdio { - child: transfer.lower(handle.as_handle())?, + child: transfer.lower(handle.borrow())?, parent: None, }), }, @@ -304,11 +292,11 @@ fn prepare_inherit( )?; match standard { Some(handle) => Ok(PreparedStdio { - child: transfer.lower(handle.as_handle())?, + child: transfer.lower(handle.borrow())?, parent: None, }), None => Ok(PreparedStdio { - child: sys::INVALID_RAW_HANDLE, + child: Value::INVALID, parent: None, }), } @@ -328,7 +316,7 @@ fn prepare_null( sys::null_handle(access), )?; Ok(PreparedStdio { - child: transfer.lower(handle.as_handle())?, + child: transfer.lower(handle.borrow())?, parent: None, }) } @@ -342,22 +330,24 @@ fn prepare_pipe( Operation::CreatePipe, sys::create_pipe(!matches!(stream, StandardStream::Input)), )?; - let child = transfer.lower(pipe.child.as_handle())?; + let child = transfer.lower(pipe.child.borrow())?; Ok(PreparedStdio { child, parent: Some(pipe.parent), }) } +/// The inheritable duplicates of one spawn, in the table the child inherits from, and their values. struct HandleTransfer<'a> { - parent: Option>, + parent: Option>, + /// Duplicates in the creator's table, which closes them when dropped. local: Vec, remote: Vec>, - inherited: Vec, + inherited: Vec>, } impl<'a> HandleTransfer<'a> { - const fn new(parent: Option>) -> Self { + const fn new(parent: Option>) -> Self { Self { parent, local: Vec::new(), @@ -366,7 +356,8 @@ impl<'a> HandleTransfer<'a> { } } - fn lower(&mut self, source: BorrowedHandle<'_>) -> Result { + /// Duplicates `source` inheritably into the inheritor's table, and returns the duplicate's value there. + fn lower(&mut self, source: Borrowed<'_, K>) -> Result> { let value = if let Some(parent) = self.parent { let handle = windows( Phase::Preparation, @@ -382,8 +373,8 @@ impl<'a> HandleTransfer<'a> { Operation::DuplicateLocalHandle, sys::duplicate_local(source, true), )?; - let value = sys::handle_value(handle.as_raw_handle()); - self.local.push(handle); + let value = Value::::of(handle.as_handle()).inherited(); + self.local.push(handle.into_inner()); value }; self.inherited.push(value); @@ -402,11 +393,13 @@ impl<'a> HandleTransfer<'a> { drop(self.local); } - const fn parent(&self) -> Option> { + /// The alternate parent's value in the creator's table, when the spawn names one. + fn parent_value(&self) -> Option> { self.parent + .map(|parent| Value::::of(parent.untyped())) } - fn inherited_values(&self) -> &[isize] { + fn inherited_values(&self) -> &[Value] { &self.inherited } } @@ -476,7 +469,7 @@ fn build_environment(command: &Command, transfer: &mut HandleTransfer<'_>) -> Re let value = match value { EnvValue::Text(value) => value.clone(), EnvValue::Handle(handle) => { - OsString::from(transfer.lower(handle.as_handle())?.to_string()) + OsString::from(transfer.lower(handle.borrow())?.to_string()) } }; match map.entry(EnvKey::new(key)) { @@ -587,7 +580,7 @@ fn build_command_line(command: &Command, transfer: &mut HandleTransfer<'_>) -> R Arg::Text(text) => append_argument(&mut result, &wide(text)), Arg::Raw(text) => result.extend(text.encode_wide()), Arg::Handle(handle) => { - let value = transfer.lower(handle.as_handle())?.to_string(); + let value = transfer.lower(handle.borrow())?.to_string(); append_argument(&mut result, &wide(OsStr::new(&value))); } } @@ -1194,10 +1187,9 @@ mod tests { ) .unwrap(); let transaction = SpawnTransaction::new(&plan).unwrap(); - let created = transaction.created.as_ref().unwrap(); - let mut process = ProcessExitGuard::watch(created.process.as_handle()); + let mut process = ProcessExitGuard::watch(transaction.child.as_handle()); assert_eq!( - suspend_count(created.thread.as_handle()), + suspend_count(transaction.main_thread()), 1, "a running spawn stays suspended until its commit" ); @@ -1225,9 +1217,8 @@ mod tests { ) .unwrap(); let transaction = SpawnTransaction::new(&plan).unwrap(); - let created = transaction.created.as_ref().unwrap(); - let mut process = ProcessExitGuard::watch(created.process.as_handle()); - let thread = duplicate(created.thread.as_handle()); + let mut process = ProcessExitGuard::watch(transaction.child.as_handle()); + let thread = duplicate(transaction.main_thread()); drop(transaction); tempt_resume(thread.as_handle()); assert_eq!( @@ -1248,9 +1239,8 @@ mod tests { ) .unwrap(); let transaction = SpawnTransaction::new(&plan).unwrap(); - let created = transaction.created.as_ref().unwrap(); - let mut process = ProcessExitGuard::watch(created.process.as_handle()); - let thread = duplicate(created.thread.as_handle()); + let mut process = ProcessExitGuard::watch(transaction.child.as_handle()); + let thread = duplicate(transaction.main_thread()); drop(transaction.commit_suspended()); tempt_resume(thread.as_handle()); assert_eq!( diff --git a/xtask/src/gates.rs b/xtask/src/gates.rs index dd4a131..b18bf7d 100644 --- a/xtask/src/gates.rs +++ b/xtask/src/gates.rs @@ -33,25 +33,6 @@ const EXCEPTIONS: &[Exception] = &[ count: 1, reason: "dword_const is evaluated in constants, where an oversized value fails the build", }, - Exception { - file: "src/child.rs", - lint: "clippy::expect_used", - count: 1, - reason: "a SuspendedChild owns its process until resume consumes it", - }, - Exception { - file: "src/transaction.rs", - lint: "clippy::expect_used", - count: 1, - reason: "an uncommitted transaction owns its process until commit consumes it", - }, - Exception { - file: "src/transaction.rs", - lint: "clippy::too_many_lines", - count: 1, - reason: - "one function acquires every creation resource, so rollback has one owner (ADR 0007)", - }, Exception { file: "tests/support/mod.rs", lint: "clippy::disallowed_methods", @@ -129,6 +110,54 @@ const DELAY_COMMANDS: [&str; 3] = [ concat!("timeout", " /t"), ]; +/// Library files that only tests compile; the library rules skip them as they skip test-only items. +const TEST_ONLY_FILES: [&str; 1] = ["src/failure_tests.rs"]; + +/// Why library code holds no shared or interior-mutable state; atomics, whose names start with `Atomic`, share it. +const SHARED_STATE: &str = + "shares or mutates state outside the borrow checker; pass owned values and borrows"; + +/// Names library code doesn't use, each set with its reason (ADR 0016). +const LIBRARY_BANS: [(&[&str], &str); 4] = [ + ( + &[ + "Rc", + "Arc", + "Weak", + "Cell", + "RefCell", + "OnceCell", + "LazyCell", + "UnsafeCell", + "Mutex", + "RwLock", + "Condvar", + "Barrier", + "atomic", + ], + SHARED_STATE, + ), + ( + &[ + "static", + "Once", + "OnceLock", + "LazyLock", + "lazy_static", + "thread_local", + ], + "keeps global state; pass the value to what needs it", + ), + ( + &["Any", "TypeId", "type_name", "type_name_of_val"], + "inspects a type at run time; dispatch on an enum or a trait bound", + ), + ( + &["HashMap", "HashSet", "RandomState"], + "iterates in a random order; use BTreeMap or BTreeSet", + ), +]; + /// A rule violation. #[derive(Debug, PartialEq, Eq)] pub(crate) struct Finding { @@ -228,6 +257,8 @@ fn check_exceptions(allows: &[Allow], exceptions: &[Exception]) -> Vec /// The offset of the token after the next one: `dyn` after `Box <`, the arguments after `name (`, and the bracket after `# !`. const AFTER_NEXT: usize = 2; +/// Two adjacent tokens, such as `fn source` read backwards. +const PAIR: usize = 2; /// The length of each comment delimiter: `//`, `/*`, and `*/`. const DELIMITER: usize = 2; /// The length of an escape: a backslash and the character it escapes. @@ -275,6 +306,8 @@ fn scan(file: &str, source: &str) -> Scan { .iter() .filter(|(_, token)| !matches!(token, Token::Comment { .. })) .collect(); + let guarded = test_only(&code); + let library = file.starts_with("src/") && !TEST_ONLY_FILES.contains(&file); let mut allows = Vec::new(); for (index, (line, token)) in code.iter().enumerate() { let next = |offset: usize| { @@ -283,6 +316,11 @@ fn scan(file: &str, source: &str) -> Scan { }; match token { Token::Ident(name) => { + if !guarded.get(index).copied().unwrap_or_default() { + if let Some(rule) = explicit_state_rule(&code, index, library) { + finding(*line, rule); + } + } if is_time_name(name) { finding( *line, @@ -355,6 +393,277 @@ fn scan(file: &str, source: &str) -> Scan { Scan { findings, allows } } +/// Returns the ADR 0016 rule that the identifier at `index` breaks. +/// +/// A trait object is refused in any file, and the library bans only in library code. +fn explicit_state_rule(code: &[&(usize, Token)], index: usize, library: bool) -> Option { + let before = |offset: usize| { + index + .checked_sub(offset) + .and_then(|at| code.get(at)) + .map(|(_, token)| token) + }; + let Some(Token::Ident(name)) = before(0) else { + return None; + }; + if name == "dyn" { + let boxed = before(AFTER_NEXT) == Some(&Token::Ident("Box".to_owned())) + && before(1) == Some(&Token::Punct('<')); + return (!boxed && !in_error_source(code, index)).then(|| { + "a trait object hides which type runs; own a closed set with an enum or an open one with a generic (ADR 0016)" + .to_owned() + }); + } + let lifetime = before(1) == Some(&Token::Punct('\'')); + if !library || lifetime { + return None; + } + library_ban(name).map(|reason| format!("`{name}` {reason} (ADR 0016)")) +} + +/// Returns why library code doesn't use `name`, if it doesn't. +fn library_ban(name: &str) -> Option<&'static str> { + if name.starts_with("Atomic") { + return Some(SHARED_STATE); + } + LIBRARY_BANS + .iter() + .find(|(names, _)| names.contains(&name)) + .map(|(_, reason)| *reason) +} + +/// Returns true if the `dyn` at `index` names the `Error` trait in the return type of `source` in an `impl … Error for …` block: +/// the `Error::source` contract. +fn in_error_source(code: &[&(usize, Token)], index: usize) -> bool { + let token_at = |at: usize| code.get(at).map(|(_, found)| found); + let names_error = code + .iter() + .skip(index.saturating_add(1)) + .map(|(_, token)| token) + .take_while(|token| matches!(token, Token::Ident(_) | Token::Punct(':'))) + .filter_map(|token| match token { + Token::Ident(name) => Some(name.as_str()), + Token::Literal(_) | Token::Str(_) | Token::Punct(_) | Token::Comment { .. } => None, + }) + .last() + == Some("Error"); + let Some(function) = (0..index) + .rev() + .take_while(|at| !matches!(token_at(*at), Some(Token::Punct('{' | '}' | ';')))) + .find(|at| matches!(token_at(*at), Some(Token::Ident(keyword)) if keyword == "fn")) + else { + return false; + }; + let name = function.saturating_add(1); + names_error + && matches!(token_at(name), Some(Token::Ident(source)) if source == "source") + && return_arrow(code, name).is_some_and(|arrow| arrow < index) + && in_error_impl(code, function) +} + +/// Returns the index of the `>` of the `->` after the parameters of the function whose name is at `name`. +fn return_arrow(code: &[&(usize, Token)], name: usize) -> Option { + let mut depth = 0_usize; + let mut close = None; + for (at, (_, token)) in code.iter().enumerate().skip(name) { + if *token == Token::Punct('(') { + depth = depth.saturating_add(1); + } else if *token == Token::Punct(')') { + depth = depth.saturating_sub(1); + if depth == 0 { + close = Some(at); + break; + } + } + } + let dash = close?.saturating_add(1); + let arrow = dash.saturating_add(1); + let punct = |at: usize, expected: char| { + code.get(at) + .is_some_and(|(_, token)| *token == Token::Punct(expected)) + }; + (punct(dash, '-') && punct(arrow, '>')).then_some(arrow) +} + +/// Returns true if the item at `item` sits in the block of an `impl … Error for …`. +fn in_error_impl(code: &[&(usize, Token)], item: usize) -> bool { + let mut depth = 0_usize; + let mut opener = None; + for (at, (_, token)) in code.iter().enumerate().take(item).rev() { + if *token == Token::Punct('}') { + depth = depth.saturating_add(1); + } else if *token == Token::Punct('{') { + if depth == 0 { + opener = Some(at); + break; + } + depth = depth.saturating_sub(1); + } + } + let Some(opener) = opener else { + return false; + }; + let header: Vec<&Token> = code + .iter() + .take(opener) + .rev() + .map(|(_, token)| token) + .take_while(|token| !matches!(token, Token::Punct('{' | '}' | ';'))) + .collect(); + header.contains(&&Token::Ident("impl".to_owned())) + && header.windows(PAIR).any(|pair| { + matches!(pair, [Token::Ident(keyword), Token::Ident(error)] if keyword == "for" && error == "Error") + }) +} + +/// Marks each token of an item, statement, or field that a test-only `cfg` attribute guards. +fn test_only(code: &[&(usize, Token)]) -> Vec { + let mut guarded = vec![false; code.len()]; + let mut index = 0_usize; + while index < code.len() { + let open = index.saturating_add(1); + let attribute = matches!(code.get(index), Some((_, Token::Punct('#')))) + && matches!(code.get(open), Some((_, Token::Punct('[')))); + if attribute && is_test_only_cfg(&bracketed(code, open)) { + let end = item_end(code, open); + for flag in guarded.iter_mut().take(end).skip(index) { + *flag = true; + } + index = end; + } else { + index = open; + } + } + guarded +} + +/// Returns true if an attribute's tokens are `cfg(test)`, `cfg(kani)`, or `cfg(any(…))` over those two. +fn is_test_only_cfg(content: &[Token]) -> bool { + let [Token::Ident(cfg), Token::Punct('('), predicate @ .., Token::Punct(')')] = content else { + return false; + }; + let members = match without_trailing_comma(predicate) { + [Token::Ident(any), Token::Punct('('), members @ .., Token::Punct(')')] if any == "any" => { + without_trailing_comma(members) + } + single => single, + }; + cfg == "cfg" + && members.split(|token| *token == Token::Punct(',')).all( + |member| matches!(member, [Token::Ident(name)] if name == "test" || name == "kani"), + ) +} + +/// Drops the one trailing comma Rust allows after the last element of a list. +fn without_trailing_comma(tokens: &[Token]) -> &[Token] { + tokens.strip_suffix(&[Token::Punct(',')]).unwrap_or(tokens) +} + +/// Returns true if `next`, the token after a closing brace, continues the statement the brace closed: +/// `else`, a method call, `?`, or the `;` or `,` that ends it. +fn continues(next: Option<&&(usize, Token)>) -> bool { + match next.map(|(_, token)| token) { + Some(Token::Ident(word)) => word == "else", + Some(Token::Punct(punct)) => matches!(punct, '.' | '?' | ';' | ','), + Some(Token::Literal(_) | Token::Str(_) | Token::Comment { .. }) | None => false, + } +} + +/// Returns true if what starts at `start`, past further attributes and a visibility, +/// is a `let` statement or a `static`, `const`, `use`, or `type` item, which only a `;` ends. +fn ends_at_semicolon(code: &[&(usize, Token)], start: usize) -> bool { + let token_at = |at: usize| code.get(at).map(|(_, found)| found); + let mut at = start; + loop { + match token_at(at) { + Some(Token::Punct('#')) => at = group_end(code, at.saturating_add(1)), + Some(Token::Ident(word)) if word == "pub" => { + at = at.saturating_add(1); + if token_at(at) == Some(&Token::Punct('(')) { + at = group_end(code, at); + } + } + Some(Token::Ident(word)) if word == "const" => { + return !matches!( + token_at(at.saturating_add(1)), + Some(Token::Ident(next)) if matches!(next.as_str(), "fn" | "unsafe" | "async" | "extern") + ); + } + Some(Token::Ident(word)) => { + return matches!(word.as_str(), "let" | "static" | "use" | "type"); + } + Some(Token::Literal(_) | Token::Str(_) | Token::Punct(_) | Token::Comment { .. }) + | None => { + return false; + } + } + } +} + +/// Returns the index after the bracket that closes the one at `open`. +fn group_end(code: &[&(usize, Token)], open: usize) -> usize { + let mut depth = 0_usize; + for (index, (_, token)) in code.iter().enumerate().skip(open) { + match token { + Token::Punct('(' | '[' | '{') => depth = depth.saturating_add(1), + Token::Punct(')' | ']' | '}') => { + depth = depth.saturating_sub(1); + if depth == 0 { + return index.saturating_add(1); + } + } + Token::Ident(_) + | Token::Literal(_) + | Token::Str(_) + | Token::Punct(_) + | Token::Comment { .. } => {} + } + } + code.len() +} + +/// Returns the index after what the attribute whose `[` is at `open` guards. +/// +/// That ends with a `;` or `,` outside brackets, which alone ends a `let` statement and a `static`, `const`, `use`, or `type` item. +/// Another item ends with a `}` that closes its body when nothing continues the statement. +/// Either ends before the bracket that closes the enclosing block. +fn item_end(code: &[&(usize, Token)], open: usize) -> usize { + let mut depth = 0_usize; + let mut in_attribute = true; + let mut semicolon = false; + for (index, (_, token)) in code.iter().enumerate().skip(open) { + match token { + Token::Punct('(' | '[' | '{') => depth = depth.saturating_add(1), + Token::Punct(close @ (')' | ']' | '}')) => { + if depth == 0 { + return index; + } + depth = depth.saturating_sub(1); + if depth == 0 { + if in_attribute { + in_attribute = false; + semicolon = ends_at_semicolon(code, index.saturating_add(1)); + } else if *close == '}' + && !semicolon + && !continues(code.get(index.saturating_add(1))) + { + return index.saturating_add(1); + } + } + } + Token::Punct(';' | ',') if depth == 0 && !in_attribute => { + return index.saturating_add(1); + } + Token::Ident(_) + | Token::Literal(_) + | Token::Str(_) + | Token::Punct(_) + | Token::Comment { .. } => {} + } + } + code.len() +} + fn is_time_name(name: &str) -> bool { let lower = name.to_ascii_lowercase(); let named = TIME_NAMES.contains(&name) @@ -810,6 +1119,132 @@ mod tests { assert_clean("struct S { f: Box }"); } + fn rules_in(file: &str, source: &str) -> Vec { + scan(file, source) + .findings + .into_iter() + .map(|finding| finding.rule) + .collect() + } + + #[test] + fn trait_objects_are_refused_outside_the_error_source_contract() { + for source in [ + "fn f(x: &dyn Fn()) {}", + "fn cause(&self) -> Option<&dyn Error> { None }", + "impl Error for E { fn source(&self) -> Option<&(dyn Display + 'static)> { None } }", + "impl Error for E { fn source(&self) {} fn g(x: &dyn Error) {} }", + "impl Error for E { fn source(&self, x: &dyn Error) {} }", + "fn source(&self) -> Option<&(dyn Error + 'static)> { None }", + "impl Display for E { fn source(&self) -> Option<&(dyn Error + 'static)> { None } }", + "m!(dyn T);", + "struct S { f: Box }", + ] { + assert_eq!(rules_in("tests/example.rs", source).len(), 1, "{source}"); + } + for source in [ + "impl Error for E { fn source(&self) -> Option<&(dyn Error + 'static)> { None } }", + "impl std::error::Error for E { fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { None } }", + "impl Error for W { fn f() {} fn source(&self) -> Option<&(dyn Error + 'static)> { None } }", + "#[cfg(test)] fn f(x: &dyn Fn()) {}", + ] { + assert_eq!( + rules_in("tests/example.rs", source), + Vec::::new(), + "{source}" + ); + } + } + + #[test] + fn library_code_keeps_no_shared_global_reflected_or_hashed_state() { + for source in [ + "use std::rc::Rc;", + "fn f() { let x = Arc::new(1); }", + "struct S { c: RefCell }", + "fn f() { let x = AtomicBool::new(false); }", + "fn f() { let m = Mutex::new(0); }", + "static X: u8 = 0;", + "thread_local! {}", + "fn f() { let l = OnceLock::new(); }", + "fn f() -> TypeId { g() }", + "fn f() { let m = HashMap::new(); }", + "#[cfg(not(test))] static X: u8 = 0;", + "#[cfg(test)] COUNT.fetch_add(1); fn f() { let s = HashSet::new(); }", + "#[cfg(test)] static X: u8 = 0; fn f() { let m = Mutex::new(0); }", + "#[cfg(test)] let x = S { a: 1 }.with(Cell::new(1)); fn f() { let m = HashMap::new(); }", + "#[cfg(test)] fn g() {} fn f() { let m = HashMap::new(); }", + "#[cfg(test)] let x = { 1 }; fn f() { let m = HashMap::new(); }", + "#[cfg(test)] const fn g() -> u8 { 1 } fn f() { let m = HashMap::new(); }", + "#[cfg(test)] pub(crate) const unsafe fn g() {} fn f() { let m = HashMap::new(); }", + ] { + assert_eq!(rules_in("src/example.rs", source).len(), 1, "{source}"); + assert_eq!(rules_in("tests/example.rs", source), Vec::::new()); + assert_eq!( + rules_in("src/failure_tests.rs", source), + Vec::::new() + ); + } + for source in [ + "fn f(x: &'static str) -> Box { g() }", + "const X: &'static str = \"\";", + "#[cfg(test)] mod tests { use std::sync::Arc; }", + "#[cfg(kani)] fn f() { let c = Cell::new(1); }", + "#[cfg(any(test, kani))] static X: u8 = 0;", + "struct S { #[cfg(test)] c: Cell, v: u8 }", + "struct S { v: u8, #[cfg(test)] c: Cell }", + "fn f() { #[cfg(test)] COUNT.fetch_add(1, atomic::Ordering::Relaxed); }", + "fn f() { #[cfg(test)] let c = if a { Cell::new(1) } else { Cell::new(2) }; }", + "fn f() { #[cfg(test)] let s = S { a: 1 }.with(RefCell::new(1))?; }", + "match x { #[cfg(test)] A => { Cell::new(1) }, B => 2 }", + "fn f() { #[cfg(test)] let value = { 1 } + Arc::strong_count(&arc); }", + "#[cfg(test)] pub(crate) static S: u8 = { 1 } + Cell::new(1).get();", + "#[cfg(test)] #[doc(hidden)] const N: usize = { 1 } + HashMap::::new().len();", + "#[cfg(test)] use std::{cell::Cell, sync::Arc};", + "#[cfg(test)] type Shared = { Arc };", + ] { + assert_eq!( + rules_in("src/example.rs", source), + Vec::::new(), + "{source}" + ); + } + } + + #[test] + fn only_cfg_test_and_kani_guard_test_code() { + let content = |source: &str| { + lex(source) + .into_iter() + .map(|(_, token)| token) + .collect::>() + }; + for guard in [ + "cfg(test)", + "cfg(kani)", + "cfg(any(test, kani))", + "cfg(any(test))", + "cfg(any(test,))", + "cfg(any(test, kani,))", + "cfg(test,)", + ] { + assert!(is_test_only_cfg(&content(guard)), "{guard}"); + } + for guard in [ + "cfg(not(test))", + "cfg(any(test, windows))", + "cfg(all(test, kani))", + "cfg()", + "cfg(any())", + "cfg(any(test,,kani))", + "cfg(any(,))", + "cfg_attr(test, allow(x))", + "test", + ] { + assert!(!is_test_only_cfg(&content(guard)), "{guard}"); + } + } + #[test] fn literals_do_not_confuse_the_lexer() { assert_clean("fn f<'a>(x: &'a str) -> char { let _ = r#\"// \"#; '\\'' }");