diff --git a/packages/zpm-switch/src/commands/switch/explicit.rs b/packages/zpm-switch/src/commands/switch/explicit.rs index c38e4dfb..08bcea72 100644 --- a/packages/zpm-switch/src/commands/switch/explicit.rs +++ b/packages/zpm-switch/src/commands/switch/explicit.rs @@ -36,13 +36,13 @@ impl ExplicitCommand { = binary.spawn() .map_err(|err| Error::FailedToExecuteBinary(binary.get_program().to_string_lossy().to_string(), Arc::new(err)))?; - // Ignore SIGINT while waiting for the child process. + // Ignore SIGINT/SIGTERM while waiting for the child process. // This ensures the child's exit code is properly propagated - // instead of the parent being killed by SIGINT. + // instead of the parent being killed by a signal. // Note: We must spawn BEFORE setting SIG_IGN, otherwise the child - // inherits the ignored signal disposition and won't receive Ctrl-C. + // inherits the ignored signal disposition and won't receive signals. #[cfg(unix)] - let _guard = zpm_utils::IgnoreSigint::new(); + let _guard = zpm_utils::IgnoreSignals::new(); let exit_code = child.wait() diff --git a/packages/zpm-utils/src/process.rs b/packages/zpm-utils/src/process.rs index a4e65c5b..07c45f61 100644 --- a/packages/zpm-utils/src/process.rs +++ b/packages/zpm-utils/src/process.rs @@ -1,49 +1,53 @@ use std::process::Command; use shlex::{try_quote, QuoteError}; -/// RAII guard to ignore SIGINT while waiting for a child process. +/// RAII guard to ignore SIGINT and SIGTERM while waiting for a child process. /// /// When a terminal user presses Ctrl-C, SIGINT is sent to the entire -/// foreground process group. If a parent process is waiting for a child, -/// both receive the signal. By ignoring SIGINT in the parent, we ensure -/// the child can handle the signal and exit gracefully, and the parent -/// can properly propagate the child's exit code. +/// foreground process group. Similarly, SIGTERM may be sent by init +/// systems (e.g. Docker/tini) to request graceful shutdown. If a parent +/// process is waiting for a child, both receive the signal. By ignoring +/// these signals in the parent, we ensure the child can handle them and +/// exit gracefully, and the parent can properly propagate the child's +/// exit code. /// -/// On drop, restores the previous signal handler. +/// On drop, restores the previous signal handlers. #[cfg(unix)] -pub struct IgnoreSigint { - prev_handler: libc::sighandler_t, +pub struct IgnoreSignals { + prev_sigint: libc::sighandler_t, + prev_sigterm: libc::sighandler_t, } #[cfg(unix)] -impl IgnoreSigint { - /// Creates a new guard that ignores SIGINT until dropped. +impl IgnoreSignals { + /// Creates a new guard that ignores SIGINT and SIGTERM until dropped. pub fn new() -> Self { // SAFETY: We're setting SIG_IGN which is always safe - let prev_handler = unsafe { libc::signal(libc::SIGINT, libc::SIG_IGN) }; + let prev_sigint = unsafe { libc::signal(libc::SIGINT, libc::SIG_IGN) }; + let prev_sigterm = unsafe { libc::signal(libc::SIGTERM, libc::SIG_IGN) }; // If signal() returns SIG_ERR, use SIG_DFL as safe fallback so Drop // restores a known-valid handler rather than attempting to set SIG_ERR. - let prev_handler = if prev_handler == libc::SIG_ERR { - libc::SIG_DFL - } else { - prev_handler - }; - Self { prev_handler } + let prev_sigint = if prev_sigint == libc::SIG_ERR { libc::SIG_DFL } else { prev_sigint }; + let prev_sigterm = if prev_sigterm == libc::SIG_ERR { libc::SIG_DFL } else { prev_sigterm }; + Self { prev_sigint, prev_sigterm } } } #[cfg(unix)] -impl Default for IgnoreSigint { +impl Default for IgnoreSignals { fn default() -> Self { Self::new() } } #[cfg(unix)] -impl Drop for IgnoreSigint { +impl Drop for IgnoreSignals { fn drop(&mut self) { - // SAFETY: We're restoring the previous handler - unsafe { libc::signal(libc::SIGINT, self.prev_handler) }; + // SAFETY: We're restoring the previous handlers + unsafe { + libc::signal(libc::SIGINT, self.prev_sigint); + libc::signal(libc::SIGTERM, self.prev_sigterm); + } } } diff --git a/packages/zpm/src/commands/dlx.rs b/packages/zpm/src/commands/dlx.rs index 57a8762f..46659c76 100644 --- a/packages/zpm/src/commands/dlx.rs +++ b/packages/zpm/src/commands/dlx.rs @@ -209,6 +209,7 @@ pub async fn run_binary(project: &Project, bin: Binary, args: Vec, curre .with_package(&project, &project.root_workspace().locator())? .with_cwd(current_cwd) .enable_shell_forwarding() + .enable_signal_delegation() .run_binary(&bin, &args) .await? .into()) diff --git a/packages/zpm/src/commands/exec.rs b/packages/zpm/src/commands/exec.rs index 379c5c2a..24ede569 100644 --- a/packages/zpm/src/commands/exec.rs +++ b/packages/zpm/src/commands/exec.rs @@ -34,6 +34,7 @@ impl Exec { .with_project(&project) .with_package(&project, &project.active_package()?)? .enable_shell_forwarding() + .enable_signal_delegation() .run_script(&self.script, &self.args) .await? .into()) diff --git a/packages/zpm/src/commands/node.rs b/packages/zpm/src/commands/node.rs index 4ca26453..04d59f41 100644 --- a/packages/zpm/src/commands/node.rs +++ b/packages/zpm/src/commands/node.rs @@ -29,6 +29,7 @@ impl Node { .with_project(&project) .with_package(&project, &project.active_package()?)? .enable_shell_forwarding() + .enable_signal_delegation() .run_exec("node", &self.args) .await? .into()) diff --git a/packages/zpm/src/commands/python.rs b/packages/zpm/src/commands/python.rs index 8c23696a..b8957ad8 100644 --- a/packages/zpm/src/commands/python.rs +++ b/packages/zpm/src/commands/python.rs @@ -100,7 +100,8 @@ impl Python { let mut env = ScriptEnvironment::new()? .with_project(&project) .with_package(&project, &project.active_package()?)? - .enable_shell_forwarding(); + .enable_shell_forwarding() + .enable_signal_delegation(); if let Some(venv_path) = active_workspace_venv(&project) { let site_packages_path diff --git a/packages/zpm/src/commands/run.rs b/packages/zpm/src/commands/run.rs index abce218f..70577103 100644 --- a/packages/zpm/src/commands/run.rs +++ b/packages/zpm/src/commands/run.rs @@ -119,6 +119,7 @@ impl Run { .with_package(&project, &project.active_package()?)? .with_node_args(get_node_args()) .enable_shell_forwarding() + .enable_signal_delegation() .run_binary(&binary, &self.args) .await? .into()) @@ -155,6 +156,7 @@ impl Run { .with_package(&project, &locator)? .with_env_variable("npm_lifecycle_event", &self.name) .enable_shell_forwarding() + .enable_signal_delegation() .run_script(&script, &self.args) .await? .into()) diff --git a/packages/zpm/src/script.rs b/packages/zpm/src/script.rs index a782471d..271872b4 100644 --- a/packages/zpm/src/script.rs +++ b/packages/zpm/src/script.rs @@ -495,12 +495,10 @@ impl ScriptEnvironment { /// Enables signal delegation mode. /// - /// When enabled, SIGINT is ignored in the parent process while waiting - /// for child processes to complete. This allows the child to handle - /// the signal and exit gracefully, with its exit code properly propagated. - /// - /// This is useful when the parent is a wrapper (like yarn-switch) that - /// should delegate signal handling to the actual command being run. + /// When enabled, SIGINT and SIGTERM are ignored in the parent process + /// while waiting for child processes to complete. This allows the child + /// to handle the signals and exit gracefully, with its exit code properly + /// propagated. pub fn enable_signal_delegation(mut self) -> Self { self.signal_delegation = true; self @@ -709,11 +707,11 @@ impl ScriptEnvironment { } } - // If signal delegation is enabled, ignore SIGINT while waiting for the child. - // This allows the child to handle the signal and exit gracefully. + // If signal delegation is enabled, ignore SIGINT/SIGTERM while waiting + // for the child. This allows the child to handle signals and exit gracefully. #[cfg(unix)] let _guard = if self.signal_delegation { - Some(zpm_utils::IgnoreSigint::new()) + Some(zpm_utils::IgnoreSignals::new()) } else { None }; @@ -835,11 +833,11 @@ impl ScriptEnvironment { cmd.stderr(std::process::Stdio::inherit()); cmd.stdin(std::process::Stdio::inherit()); - // If signal delegation is enabled, ignore SIGINT while waiting for the child. - // This allows the child to handle the signal and exit gracefully. + // If signal delegation is enabled, ignore SIGINT/SIGTERM while waiting + // for the child. This allows the child to handle signals and exit gracefully. #[cfg(unix)] let _guard = if self.signal_delegation { - Some(zpm_utils::IgnoreSigint::new()) + Some(zpm_utils::IgnoreSignals::new()) } else { None }; diff --git a/tests/acceptance-tests/pkg-tests-specs/sources/commands/switch/proxy.test.ts b/tests/acceptance-tests/pkg-tests-specs/sources/commands/switch/proxy.test.ts index 4a015980..b813b3e9 100644 --- a/tests/acceptance-tests/pkg-tests-specs/sources/commands/switch/proxy.test.ts +++ b/tests/acceptance-tests/pkg-tests-specs/sources/commands/switch/proxy.test.ts @@ -1,5 +1,6 @@ import {npath, ppath, xfs} from '@yarnpkg/fslib'; import {spawn} from 'child_process'; +import {delimiter} from 'path'; import {RunFunction} from '../../../../pkg-tests-core/sources/utils/tests'; @@ -13,6 +14,14 @@ function cleanupDaemon(cb: RunFunction): RunFunction { }; } +const getSwitchBinaryPath = () => + process.env.TEST_SWITCH_BINARY + ?? require.resolve(`${__dirname}/../../../../../../target/release/yarn`); + +const getYarnBinBinaryPath = () => + process.env.TEST_BINARY + ?? require.resolve(`${__dirname}/../../../../../../target/release/yarn-bin`); + describe(`Commands`, () => { describe(`switch proxy`, () => { test( @@ -69,5 +78,139 @@ describe(`Commands`, () => { expect(exitCode).toBe(0); })), ); + + test( + `it should delegate SIGINT to child when sent to the process group`, + makeTemporaryEnv({ + name: `test-package`, + }, async ({path, run}) => { + await xfs.writeFilePromise(ppath.join(path, `signal-test.js`), [ + `process.on("SIGINT", () => {`, + ` process.stdout.write("SIGINT handled\\n");`, + ` process.exit(0);`, + `});`, + `process.stdout.write("ready\\n");`, + `setInterval(() => {}, 1000);`, + ].join(`\n`)); + + await run(`install`); + + const nativePath = npath.fromPortablePath(path); + const switchBinary = getSwitchBinaryPath(); + const yarnBinBinary = getYarnBinBinaryPath(); + + const child = spawn(switchBinary, [`node`, `signal-test.js`], { + cwd: nativePath, + detached: true, + stdio: [`ignore`, `pipe`, `pipe`], + env: { + HOME: npath.dirname(nativePath), + PATH: `${nativePath}/bin${delimiter}${process.env.PATH}`, + YARN_IS_TEST_ENV: `true`, + YARN_GLOBAL_FOLDER: `${nativePath}/.yarn/global`, + YARN_ENABLE_TELEMETRY: `0`, + YARN_ENABLE_PROGRESS_BARS: `false`, + YARN_ENABLE_TIMERS: `false`, + FORCE_COLOR: `0`, + NODE_OPTIONS: ``, + YARNSW_DEFAULT: `local:${yarnBinBinary}`, + }, + }); + + await new Promise((resolve, reject) => { + const timeout = setTimeout(() => { + reject(new Error(`Timeout waiting for child to be ready`)); + }, 10000); + + child.stdout?.on(`data`, (data: Buffer) => { + if (data.toString().includes(`ready`)) { + clearTimeout(timeout); + resolve(); + } + }); + + child.on(`error`, err => { + clearTimeout(timeout); + reject(err); + }); + }); + + // Send SIGINT to the entire process group (simulates Ctrl-C) + process.kill(-child.pid!, `SIGINT`); + + const exitCode = await new Promise(resolve => { + child.on(`close`, code => resolve(code)); + }); + + expect(exitCode).toBe(0); + }), + ); + + test( + `it should delegate SIGTERM to child when sent to the process group`, + makeTemporaryEnv({ + name: `test-package`, + }, async ({path, run}) => { + await xfs.writeFilePromise(ppath.join(path, `signal-test.js`), [ + `process.on("SIGTERM", () => {`, + ` process.stdout.write("SIGTERM handled\\n");`, + ` process.exit(0);`, + `});`, + `process.stdout.write("ready\\n");`, + `setInterval(() => {}, 1000);`, + ].join(`\n`)); + + await run(`install`); + + const nativePath = npath.fromPortablePath(path); + const switchBinary = getSwitchBinaryPath(); + const yarnBinBinary = getYarnBinBinaryPath(); + + const child = spawn(switchBinary, [`node`, `signal-test.js`], { + cwd: nativePath, + detached: true, + stdio: [`ignore`, `pipe`, `pipe`], + env: { + HOME: npath.dirname(nativePath), + PATH: `${nativePath}/bin${delimiter}${process.env.PATH}`, + YARN_IS_TEST_ENV: `true`, + YARN_GLOBAL_FOLDER: `${nativePath}/.yarn/global`, + YARN_ENABLE_TELEMETRY: `0`, + YARN_ENABLE_PROGRESS_BARS: `false`, + YARN_ENABLE_TIMERS: `false`, + FORCE_COLOR: `0`, + NODE_OPTIONS: ``, + YARNSW_DEFAULT: `local:${yarnBinBinary}`, + }, + }); + + await new Promise((resolve, reject) => { + const timeout = setTimeout(() => { + reject(new Error(`Timeout waiting for child to be ready`)); + }, 10000); + + child.stdout?.on(`data`, (data: Buffer) => { + if (data.toString().includes(`ready`)) { + clearTimeout(timeout); + resolve(); + } + }); + + child.on(`error`, err => { + clearTimeout(timeout); + reject(err); + }); + }); + + // Send SIGTERM to the entire process group (simulates docker stop / tini) + process.kill(-child.pid!, `SIGTERM`); + + const exitCode = await new Promise(resolve => { + child.on(`close`, code => resolve(code)); + }); + + expect(exitCode).toBe(0); + }), + ); }); });