From 9539fea16e0d8d07faf7edf50d480e0a80c4ccfc Mon Sep 17 00:00:00 2001 From: nsfisis Date: Tue, 11 Aug 2026 12:19:31 +0900 Subject: feat(signal): abort on SIGINT, SIGTERM and SIGHUP at checkpoints The SignalHandler port was a no-op stub, so all four of Composer's abort paths were dead code: nothing removed a half-created project, reverted composer.json, or cleaned up half-installed packages. Composer runs those handlers from pcntl callbacks, which a Rust signal handler cannot do -- it may touch nothing beyond atomics. SignalSubscription records the signal instead, and the abort runs from checkpoints on the normal call stack, where the clean-up can borrow the state it needs. That also resolves the closure-capture TODO(phase-c)s in RequireCommand and InstallationManager, and replaces exit_with_last_signal's exit(0) with the restore-and-re-raise Seld\Signal does. A subscription is live only inside the four abort regions, so elsewhere the signals keep their default disposition and kill the process at once. It is installed without SA_RESTART so a signal interrupts an interactive prompt rather than resuming the read. A signal reaches only the innermost subscription, reproducing SignalHandler's single-stack dispatch. Drop SignalRegistry, SignalableCommandInterface and the Application wiring for them: nothing in Composer reaches that path, and SignalHandler discards whatever they register. Signal handling from plugins and scripts is undefined behavior; see docs/dev/signals.md. Co-Authored-By: Claude Opus 5 (1M context) --- .../shirabe/src/command/create_project_command.rs | 61 ++++++++++++---------- crates/shirabe/src/command/require_command.rs | 47 +++++++++-------- 2 files changed, 59 insertions(+), 49 deletions(-) (limited to 'crates/shirabe/src/command') diff --git a/crates/shirabe/src/command/create_project_command.rs b/crates/shirabe/src/command/create_project_command.rs index 787e31d7..c7768f11 100644 --- a/crates/shirabe/src/command/create_project_command.rs +++ b/crates/shirabe/src/command/create_project_command.rs @@ -32,6 +32,7 @@ use crate::repository::PlatformRepository; use crate::repository::RepositoryFactory; use crate::repository::RepositorySet; use crate::script::ScriptEvents; +use crate::signal::SignalSubscription; use crate::util::Filesystem; use crate::util::Platform; use crate::util::ProcessExecutor; @@ -43,7 +44,6 @@ use shirabe_php_shim::{ chdir, explode_with_limit, file_exists, getcwd, impl_php_class, implode, is_dir, is_file, mkdir, realpath, rtrim, strtolower, unlink, }; -use shirabe_seld_signal::SignalHandler; use shirabe_symfony_console::command::Command; use shirabe_symfony_console::input::InputInterface; use shirabe_symfony_console::output::OutputInterface; @@ -731,28 +731,23 @@ impl CreateProjectCommand { // handler Ctrl+C aborts gracefully let _ = mkdir(&directory, 0o777, true); - let mut signal_handler: Option = None; - if let Some(real_dir) = realpath(&directory) { - let real_dir_clone = real_dir; - let io_for_signal = io.clone(); - signal_handler = Some(SignalHandler::create( - vec![ - SignalHandler::SIGINT.to_string(), - SignalHandler::SIGTERM.to_string(), - SignalHandler::SIGHUP.to_string(), - ], - Box::new(move |signal: String, handler: &SignalHandler| { - io_for_signal.write_error3( - &format!("Received {}, aborting", signal), - true, - crate::io::DEBUG, - ); - let mut fs = Filesystem::new(None); - fs.remove_directory(&real_dir_clone).ok(); - handler.exit_with_last_signal(); - }), - )); - } + let real_dir = realpath(&directory); + let signals = real_dir.as_ref().map(|_| SignalSubscription::new()); + let abort_on_signal = |signals: &SignalSubscription| { + io.write_error3( + &format!("Received {}, aborting", signals.last_signal().as_str()), + true, + crate::io::DEBUG, + ); + let mut fs = Filesystem::new(None); + fs.remove_directory( + real_dir + .as_ref() + .expect("subscribed only when realpath succeeded"), + ) + .ok(); + signals.exit_with_last_signal(); + }; // avoid displaying 9999999-dev as version if default-branch was selected if let Some(alias) = package.as_alias() @@ -791,13 +786,19 @@ impl CreateProjectCommand { ); // A shared borrow: plugin registration inside execute re-enters this manager handle // through the Composer graph. - installation_manager.borrow().execute( + let executed = installation_manager.borrow().execute( &installed_repo, vec![InstallOperation::new(package.clone()).into()], true, true, false, - )?; + ); + if let Some(signals) = &signals + && signals.is_triggered() + { + abort_on_signal(signals); + } + executed?; installation_manager .borrow_mut() .notify_installs(io.clone()); @@ -826,11 +827,15 @@ impl CreateProjectCommand { Platform::put_env("COMPOSER_ROOT_VERSION", &package.get_pretty_version()); - // once the root project is fully initialized, we do not need to wipe everything on user abort anymore even if it happens during deps install - if let Some(handler) = signal_handler { - handler.unregister(); + if let Some(signals) = &signals + && signals.is_triggered() + { + abort_on_signal(signals); } + // once the root project is fully initialized, we do not need to wipe everything on user abort anymore even if it happens during deps install + drop(signals); + Ok(installed_from_vcs) } } diff --git a/crates/shirabe/src/command/require_command.rs b/crates/shirabe/src/command/require_command.rs index d4295a50..f06c4d84 100644 --- a/crates/shirabe/src/command/require_command.rs +++ b/crates/shirabe/src/command/require_command.rs @@ -28,6 +28,7 @@ use crate::repository::CompositeRepository; use crate::repository::PlatformRepository; use crate::repository::PlatformRepositoryHandle; use crate::repository::RepositorySet; +use crate::signal::SignalSubscription; use crate::util::Filesystem; use crate::util::PackageSorter; use crate::util::Silencer; @@ -37,7 +38,6 @@ use shirabe_php_shim::{ array_merge, array_unique, empty, file_exists, file_get_contents, file_put_contents, filesize, impl_php_class, implode, is_writable, strtolower, unlink, }; -use shirabe_seld_signal::SignalHandler; use shirabe_symfony_console::command::Command; use shirabe_symfony_console::input::InputInterface; use shirabe_symfony_console::output::OutputInterface; @@ -839,25 +839,16 @@ impl Command for RequireCommand { None }; - // PHP: function ($signal, $handler) use ($io, $self) { - // $io->writeError('Received '.$signal.', aborting', true, IOInterface::DEBUG); - // $self->revertComposerFile(); $handler->exitWithLastSignal(); } - // TODO(phase-c): SignalHandler::create takes a `Box + 'static` handler that cannot - // borrow &self, but the body must call self.revert_composer_file() (which mutates the - // command's composer.json backup state) and self.get_io(). Faithfully wiring this needs the - // revert state + io shared into the closure (Rc>), i.e. the shared-ownership - // rework of the command — the same pattern as InstallationManager::execute's signal handler. - let signal_handler = SignalHandler::create( - vec![ - SignalHandler::SIGINT.to_string(), - SignalHandler::SIGTERM.to_string(), - SignalHandler::SIGHUP.to_string(), - ], - Box::new(move |signal: String, handler: &SignalHandler| { - let _ = signal; - handler.exit_with_last_signal(); - }), - ); + let signals = SignalSubscription::new(); + let abort_on_signal = |signals: &SignalSubscription| { + self.get_io().write_error3( + &format!("Received {}, aborting", signals.last_signal().as_str()), + true, + io_interface::DEBUG, + ); + self.revert_composer_file(); + signals.exit_with_last_signal(); + }; // check for writability by writing to the file as is_writable can not be trusted on network-mounts // see https://github.com/composer/composer/issues/8231 and https://bugs.php.net/bug.php?id=68926 @@ -971,6 +962,10 @@ impl Command for RequireCommand { fixed, ); + if signals.is_triggered() { + abort_on_signal(&signals); + } + let requirements = match requirements_result { Ok(r) => r, Err(e) => { @@ -1192,6 +1187,10 @@ impl Command for RequireCommand { self.update_file(&json, &requirements, require_key, remove_key, sort_packages); } + if signals.is_triggered() { + abort_on_signal(&signals); + } + let updated_msg = format!( "{} has been {}", file, @@ -1227,6 +1226,9 @@ impl Command for RequireCommand { require_key, remove_key, ); + if signals.is_triggered() { + abort_on_signal(&signals); + } let dry_run = input .borrow() .get_option("dry-run")? @@ -1262,12 +1264,15 @@ impl Command for RequireCommand { } }; + if signals.is_triggered() { + abort_on_signal(&signals); + } + // finally if dry_run && self.newly_created.get() { // @unlink($this->json->getPath()); unlink(json.borrow().get_path()); } - signal_handler.unregister(); result } -- cgit v1.3.1-4-g156e