[cargo-nextest] add the experimental ability to double-spawn test processes

We're going to need this to support SIGTSTP properly, as well as for PID
namespacing (#624). Add an experimental implementation gated behind an
env var.

Author: sunshowers

.github/workflows/ci.yml

@@ -90,6 +90,11 @@ jobs:
         uses: taiki-e/install-action@nextest
       - name: Test with latest nextest release
         run: cargo nextest run --profile ci
+      - name: Test double-spawning test processes
+        if: ${{ matrix.os == 'ubuntu-latest' || matrix.os == 'macos-latest' }}
+        env:
+          NEXTEST_EXPERIMENTAL_DOUBLE_SPAWN: 1
+        run: cargo local-nt run --profile ci
 
   test-archive-target-runner:
     name: Test archives with a target runner

cargo-nextest/src/dispatch.rs

@@ -19,8 +19,12 @@ use nextest_runner::{
         get_num_cpus, NextestConfig, NextestProfile, PreBuildPlatform, RetryPolicy, TestThreads,
         ToolConfigFile,
     },
+    double_spawn::DoubleSpawnInfo,
     errors::WriteTestListError,
-    list::{BinaryList, OutputFormat, RustTestArtifact, SerializableFormat, TestList},
+    list::{
+        BinaryList, OutputFormat, RustTestArtifact, SerializableFormat, TestExecuteContext,
+        TestList,
+    },
     partition::PartitionerBuilder,
     platform::BuildPlatforms,
     reporter::{FinalStatusLevel, StatusLevel, TestOutputDisplay, TestReporterBuilder},
@@ -53,15 +57,22 @@ pub struct CargoNextestApp {
 impl CargoNextestApp {
     /// Executes the app.
     pub fn exec(self, output_writer: &mut OutputWriter) -> Result<i32> {
-        let NextestSubcommand::Nextest(app) = self.subcommand;
-        app.exec(output_writer)
+        match self.subcommand {
+            NextestSubcommand::Nextest(app) => app.exec(output_writer),
+            #[cfg(unix)]
+            NextestSubcommand::DoubleSpawn(opts) => opts.exec(),
+        }
     }
 }
 
 #[derive(Debug, Subcommand)]
 enum NextestSubcommand {
     /// A next-generation test runner for Rust. <https://nexte.st>
-    Nextest(AppOpts),
+    Nextest(Box<AppOpts>),
+    /// Private command, used to double-spawn test processes.
+    #[cfg(unix)]
+    #[command(name = nextest_runner::double_spawn::DoubleSpawnInfo::SUBCOMMAND_NAME, hide = true)]
+    DoubleSpawn(crate::double_spawn::DoubleSpawnOpts),
 }
 
 #[derive(Debug, Args)]
@@ -442,10 +453,10 @@ struct TestBuildFilter {
 impl TestBuildFilter {
     fn compute_test_list<'g>(
         &self,
+        ctx: &TestExecuteContext<'_>,
         graph: &'g PackageGraph,
         binary_list: Arc<BinaryList>,
         test_filter_builder: TestFilterBuilder,
-        runner: &TargetRunner,
         env: EnvironmentMap,
         reuse_build: &ReuseBuildInfo,
     ) -> Result<TestList<'g>> {
@@ -464,10 +475,10 @@ impl TestBuildFilter {
             self.platform_filter.into(),
         )?;
         TestList::new(
+            ctx,
             test_artifacts,
             rust_build_meta,
             &test_filter_builder,
-            runner,
             env,
             // TODO: do we need to allow customizing this?
             get_num_cpus(),
@@ -835,6 +846,7 @@ struct BaseApp {
     config_opts: ConfigOpts,
 
     cargo_configs: CargoConfigs,
+    double_spawn: OnceCell<DoubleSpawnInfo>,
     target_runner: OnceCell<TargetRunner>,
 }
 
@@ -918,10 +930,22 @@ impl BaseApp {
             config_opts,
             cargo_configs,
 
+            double_spawn: OnceCell::new(),
             target_runner: OnceCell::new(),
         })
     }
 
+    fn load_double_spawn(&self) -> &DoubleSpawnInfo {
+        self.double_spawn.get_or_init(|| {
+            if std::env::var("NEXTEST_EXPERIMENTAL_DOUBLE_SPAWN") == Ok("1".to_owned()) {
+                log::info!("using experimental double-spawn method for test processes");
+                DoubleSpawnInfo::enabled()
+            } else {
+                DoubleSpawnInfo::disabled()
+            }
+        })
+    }
+
     fn load_runner(&self, build_platforms: &BuildPlatforms) -> &TargetRunner {
         self.target_runner
             .get_or_init(|| runner_for_target(&self.cargo_configs, build_platforms))
@@ -1039,16 +1063,16 @@ impl App {
 
     fn build_test_list(
         &self,
+        ctx: &TestExecuteContext<'_>,
         binary_list: Arc<BinaryList>,
         test_filter_builder: TestFilterBuilder,
-        target_runner: &TargetRunner,
     ) -> Result<TestList> {
         let env = EnvironmentMap::new(&self.base.cargo_configs);
         self.build_filter.compute_test_list(
+            ctx,
             self.base.graph(),
             binary_list,
             test_filter_builder,
-            target_runner,
             env,
             &self.base.reuse_build,
         )
@@ -1101,11 +1125,16 @@ impl App {
                 writer.flush().map_err(WriteTestListError::Io)?;
             }
             ListType::Full => {
+                let double_spawn = self.base.load_double_spawn();
                 let target_runner = self
                     .base
                     .load_runner(&binary_list.rust_build_meta.build_platforms()?);
-                let test_list =
-                    self.build_test_list(binary_list, test_filter_builder, target_runner)?;
+                let ctx = TestExecuteContext {
+                    double_spawn,
+                    target_runner,
+                };
+
+                let test_list = self.build_test_list(&ctx, binary_list, test_filter_builder)?;
 
                 let mut writer = output_writer.stdout_writer();
                 test_list.write(
@@ -1138,9 +1167,14 @@ impl App {
 
         let binary_list = self.base.build_binary_list()?;
         let build_platforms = binary_list.rust_build_meta.build_platforms()?;
+        let double_spawn = self.base.load_double_spawn();
         let target_runner = self.base.load_runner(&build_platforms);
+        let ctx = TestExecuteContext {
+            double_spawn,
+            target_runner,
+        };
 
-        let test_list = self.build_test_list(binary_list, test_filter_builder, target_runner)?;
+        let test_list = self.build_test_list(&ctx, binary_list, test_filter_builder)?;
 
         let output = output_writer.reporter_output();
         let profile = profile.apply_build_platforms(&build_platforms);
@@ -1162,8 +1196,13 @@ impl App {
             }
         };
 
-        let mut runner =
-            runner_builder.build(&test_list, profile, handler, target_runner.clone())?;
+        let mut runner = runner_builder.build(
+            &test_list,
+            profile,
+            handler,
+            double_spawn.clone(),
+            target_runner.clone(),
+        )?;
 
         configure_handle_inheritance(no_capture)?;
         let run_stats = runner.try_execute(|event| {

cargo-nextest/src/double_spawn.rs

@@ -0,0 +1,33 @@
+// Copyright (c) The nextest Contributors
+// SPDX-License-Identifier: MIT OR Apache-2.0
+
+use crate::{output::Color, ExpectedError, Result};
+use camino::Utf8PathBuf;
+use clap::Args;
+use std::os::unix::process::CommandExt;
+
+#[derive(Debug, Args)]
+pub(crate) struct DoubleSpawnOpts {
+    /// The program to execute.
+    program: Utf8PathBuf,
+
+    /// The args to execute the program with, provided as a string.
+    args: String,
+}
+
+impl DoubleSpawnOpts {
+    pub(crate) fn exec(self) -> Result<i32> {
+        // This double-spawned process should never use coloring.
+        Color::Never.init();
+        let args = shell_words::split(&self.args).map_err(|err| {
+            ExpectedError::DoubleSpawnParseArgsError {
+                args: self.args,
+                err,
+            }
+        })?;
+        let mut command = std::process::Command::new(&self.program);
+        // Note: exec only returns an error -- in the success case it never returns.
+        let err = command.args(args).exec();
+        Err(ExpectedError::DoubleSpawnExecError { command, err })
+    }
+}

cargo-nextest/src/errors.rs

@@ -192,6 +192,18 @@ pub enum ExpectedError {
         reason: &'static str,
         args: Vec<String>,
     },
+    #[error("double-spawn parse error")]
+    DoubleSpawnParseArgsError {
+        args: String,
+        #[source]
+        err: shell_words::ParseError,
+    },
+    #[error("double-spawn execution error")]
+    DoubleSpawnExecError {
+        command: std::process::Command,
+        #[source]
+        err: std::io::Error,
+    },
 }
 
 impl ExpectedError {
@@ -319,6 +331,9 @@ impl ExpectedError {
             | Self::SignalHandlerSetupError { .. } => NextestExitCode::SETUP_ERROR,
             #[cfg(feature = "self-update")]
             Self::UpdateVersionParseError { .. } => NextestExitCode::SETUP_ERROR,
+            Self::DoubleSpawnParseArgsError { .. } | Self::DoubleSpawnExecError { .. } => {
+                NextestExitCode::DOUBLE_SPAWN_ERROR
+            }
             Self::FromMessagesError { .. } | Self::CreateTestListError { .. } => {
                 NextestExitCode::TEST_LIST_CREATION_FAILED
             }
@@ -591,6 +606,14 @@ impl ExpectedError {
                 );
                 None
             }
+            Self::DoubleSpawnParseArgsError { args, err } => {
+                log::error!("[double-spawn] failed to parse arguments `{args}`");
+                Some(err as &dyn Error)
+            }
+            Self::DoubleSpawnExecError { command, err } => {
+                log::error!("[double-spawn] failed to exec `{command:?}`");
+                Some(err as &dyn Error)
+            }
         };
 
         while let Some(err) = next_error {

cargo-nextest/src/lib.rs

@@ -15,6 +15,8 @@
 
 mod cargo_cli;
 mod dispatch;
+#[cfg(unix)]
+mod double_spawn;
 mod errors;
 mod output;
 mod reuse_build;

cargo-nextest/src/output.rs

@@ -68,7 +68,7 @@ impl Default for Color {
 static INIT_LOGGER: std::sync::Once = std::sync::Once::new();
 
 impl Color {
-    fn init(self) {
+    pub(crate) fn init(self) {
         match self {
             Color::Auto => owo_colors::unset_override(),
             Color::Always => owo_colors::set_override(true),

nextest-metadata/src/exit_codes.rs

@@ -16,6 +16,9 @@ impl NextestExitCode {
     /// Building tests produced an error.
     pub const BUILD_FAILED: i32 = 101;
 
+    /// An error was encountered while attempting to double-spawn a nextest process.
+    pub const DOUBLE_SPAWN_ERROR: i32 = 70;
+
     /// One or more tests failed.
     pub const TEST_RUN_FAILED: i32 = 100;
 

nextest-runner/src/double_spawn.rs

@@ -0,0 +1,110 @@
+// Copyright (c) The nextest Contributors
+// SPDX-License-Identifier: MIT OR Apache-2.0
+
+//! Support for double-spawning test processes.
+//!
+//! Nextest has experimental support on Unix for spawning test processes twice, to enable better
+//! isolation and solve some thorny issues.
+
+use self::imp::DoubleSpawnInfoImp;
+use std::path::Path;
+
+/// Information about double-spawning processes. This determines whether a process will be
+/// double-spawned.
+///
+/// This is used by the main nextest process.
+#[derive(Clone, Debug)]
+pub struct DoubleSpawnInfo {
+    inner: DoubleSpawnInfoImp,
+}
+
+impl DoubleSpawnInfo {
+    /// The name of the double-spawn subcommand, used throughout nextest.
+    pub const SUBCOMMAND_NAME: &'static str = "__double-spawn";
+
+    /// This returns a `DoubleSpawnInfo` which attempts to perform double-spawning.
+    ///
+    /// This is super experimental, and should be used with caution.
+    pub fn enabled() -> Self {
+        Self {
+            inner: DoubleSpawnInfoImp::enabled(),
+        }
+    }
+
+    /// This returns a `DoubleSpawnInfo` which disables double-spawning.
+    pub fn disabled() -> Self {
+        Self {
+            inner: DoubleSpawnInfoImp::disabled(),
+        }
+    }
+    /// Returns the current executable, if one is available.
+    ///
+    /// If `None`, double-spawning is not used.
+    pub fn current_exe(&self) -> Option<&Path> {
+        self.inner.current_exe()
+    }
+}
+
+#[cfg(unix)]
+mod imp {
+    use super::*;
+    use std::path::PathBuf;
+
+    #[derive(Clone, Debug)]
+    pub(super) struct DoubleSpawnInfoImp {
+        current_exe: Option<PathBuf>,
+    }
+
+    impl DoubleSpawnInfoImp {
+        #[inline]
+        pub(super) fn enabled() -> Self {
+            // Attempt to obtain the current exe, and warn if it couldn't be found.
+            // TODO: maybe add an option to fail?
+            let current_exe = std::env::current_exe().map_or_else(
+                |error| {
+                    log::warn!(
+                        "unable to determine current exe, tests will be less isolated: {error}"
+                    );
+                    None
+                },
+                Some,
+            );
+            Self { current_exe }
+        }
+
+        #[inline]
+        pub(super) fn disabled() -> Self {
+            Self { current_exe: None }
+        }
+
+        #[inline]
+        pub(super) fn current_exe(&self) -> Option<&Path> {
+            self.current_exe.as_deref()
+        }
+    }
+}
+
+#[cfg(not(unix))]
+mod imp {
+    use super::*;
+
+    #[derive(Clone, Debug)]
+    pub(super) struct DoubleSpawnInfoImp {}
+
+    impl DoubleSpawnInfoImp {
+        #[inline]
+        pub(super) fn enabled() -> Self {
+            Self {}
+        }
+
+        #[inline]
+        pub(super) fn disabled() -> Self {
+            Self {}
+        }
+
+        #[inline]
+        pub(super) fn current_exe(&self) -> Option<&Path> {
+            None
+        }
+    }
+}