diff options
Diffstat (limited to 'crates/shirabe/src')
| -rw-r--r-- | crates/shirabe/src/command/create_project_command.rs | 61 | ||||
| -rw-r--r-- | crates/shirabe/src/command/require_command.rs | 47 | ||||
| -rw-r--r-- | crates/shirabe/src/console/application.rs | 63 | ||||
| -rw-r--r-- | crates/shirabe/src/installer/installation_manager.rs | 71 | ||||
| -rw-r--r-- | crates/shirabe/src/lib.rs | 1 | ||||
| -rw-r--r-- | crates/shirabe/src/signal.rs | 205 | ||||
| -rw-r--r-- | crates/shirabe/src/util/process_executor.rs | 37 |
7 files changed, 340 insertions, 145 deletions
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<SignalHandler> = 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<dyn Fn> + '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<RefCell<...>>), 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!( "<info>{} has been {}</info>", 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 } diff --git a/crates/shirabe/src/console/application.rs b/crates/shirabe/src/console/application.rs index c0cd072b..197ddfbd 100644 --- a/crates/shirabe/src/console/application.rs +++ b/crates/shirabe/src/console/application.rs @@ -69,7 +69,6 @@ use shirabe_seld_json_lint::ParsingException; use shirabe_symfony_console::application::Application as BaseApplication; use shirabe_symfony_console::command::Command as SymfonyCommand; use shirabe_symfony_console::command::HelpCommand; -use shirabe_symfony_console::command::SignalableCommandInterface; use shirabe_symfony_console::command_loader::CommandLoaderInterface; use shirabe_symfony_console::completion::CompletionInput; use shirabe_symfony_console::completion::CompletionSuggestions; @@ -94,7 +93,6 @@ use shirabe_symfony_console::input::InputOption; use shirabe_symfony_console::output::ConsoleOutput; use shirabe_symfony_console::output::ConsoleOutputInterface; use shirabe_symfony_console::output::{OutputInterface, output_interface}; -use shirabe_symfony_console::signal_registry::SignalRegistry; use shirabe_symfony_console::style::StyleInterface; use shirabe_symfony_console::style::SymfonyStyle; use shirabe_symfony_console::terminal::Terminal; @@ -103,6 +101,10 @@ use shirabe_symfony_process::exception::ProcessTimedOutException; /// The PHP `Composer\Console\Application` and `Symfony\Component\Console\Application` are /// flattened into a single struct. Methods that are overridden by subclass and called via /// `parent::` are prefixed by `base_`. +/// +/// Symfony's signal dispatch — `$signalRegistry`, `$signalsToDispatchEvent`, and the +/// `SignalableCommandInterface` branch of `doRunCommand` — is unported. See +/// `docs/dev/signals.md`. #[derive(Debug)] pub struct Application { commands: IndexMap<String, std::rc::Rc<std::cell::RefCell<dyn SymfonyCommand>>>, @@ -117,8 +119,6 @@ pub struct Application { terminal: Terminal, default_command: String, single_command: bool, - signal_registry: Option<SignalRegistry>, - signals_to_dispatch_event: Vec<i64>, // $initialized is omitted. See ApplicationHandle::init(). pub(crate) composer: Option<PartialComposerHandle>, pub(crate) io: std::rc::Rc<std::cell::RefCell<dyn IOInterface>>, @@ -175,7 +175,7 @@ impl Application { let initial_working_directory = getcwd(); - let mut this = Self { + Self { commands: IndexMap::new(), want_helps: false, running_command: None, @@ -188,8 +188,6 @@ impl Application { terminal: Terminal::new(), default_command: "list".to_string(), single_command: false, - signal_registry: None, - signals_to_dispatch_event: Vec::new(), composer: None, io, has_plugin_commands: false, @@ -199,17 +197,7 @@ impl Application { initial_working_directory, dev_warning_time: composer::COMPOSER_DEV_WARNING_TIME, me: std::rc::Weak::new(), - }; - if defined("SIGINT") && SignalRegistry::is_supported() { - this.signal_registry = Some(SignalRegistry::new()); - this.signals_to_dispatch_event = vec![ - shirabe_php_shim::SIGINT, - shirabe_php_shim::SIGTERM, - shirabe_php_shim::SIGUSR1, - shirabe_php_shim::SIGUSR2, - ]; } - this } /// Returns the shared handle to this application set up by `ApplicationHandle::new`. Proxy @@ -731,18 +719,6 @@ impl Application { self.command_loader = Some(command_loader); } - pub fn get_signal_registry(&self) -> anyhow::Result<&SignalRegistry> { - match &self.signal_registry { - None => Err(ConsoleRuntimeException::new("Signals are not supported. Make sure that the `pcntl` extension is installed and that \"pcntl_*\" functions are not disabled by your php.ini's \"disable_functions\" directive.".to_string()) - .into()), - Some(signal_registry) => Ok(signal_registry), - } - } - - pub fn set_signals_to_dispatch_event(&mut self, signals_to_dispatch_event: Vec<i64>) { - self.signals_to_dispatch_event = signals_to_dispatch_event; - } - pub fn set_helper_set(&mut self, helper_set: std::rc::Rc<std::cell::RefCell<HelperSet>>) { self.helper_set = Some(helper_set); } @@ -2993,35 +2969,6 @@ impl ApplicationHandle { // be reached until dynamic helper registration is restored (see HelperSet). let _ = command.borrow().get_helper_set(); - if !application.borrow().signals_to_dispatch_event.is_empty() { - // $commandSignals = $command instanceof SignalableCommandInterface ? $command->getSubscribedSignals() : [] - // TODO(phase-c): SymfonyCommand is not a SignalableCommandInterface here; downcast needed. - let command_signals: Vec<i64> = Vec::new(); - let _ = std::marker::PhantomData::<dyn SignalableCommandInterface>; - - if !command_signals.is_empty() { - if application.borrow().signal_registry.is_none() { - return Err(ConsoleRuntimeException::new("Unable to subscribe to signal events. Make sure that the `pcntl` extension is installed and that \"pcntl_*\" functions are not disabled by your php.ini's \"disable_functions\" directive.".to_string()) - .into()); - } - - if Terminal::has_stty_available() { - // TODO(phase-c): registers SIGINT/SIGTERM handlers that restore the stty mode via - // shell_exec('stty ...'). pcntl signal handlers have no faithful Rust - // equivalent yet. - let _stty_mode = shirabe_php_shim::shell_exec("stty -g"); - for _signal in [shirabe_php_shim::SIGINT, shirabe_php_shim::SIGTERM] { - todo!("register signal handler to restore stty mode"); - } - } - } - - for _signal in command_signals { - // $this->signalRegistry->register($signal, [$command, 'handleSignal']); - todo!("register command->handle_signal as signal handler"); - } - } - command .borrow() .run(input.clone(), output.clone()) diff --git a/crates/shirabe/src/installer/installation_manager.rs b/crates/shirabe/src/installer/installation_manager.rs index 24960d28..7fdec9e1 100644 --- a/crates/shirabe/src/installer/installation_manager.rs +++ b/crates/shirabe/src/installer/installation_manager.rs @@ -17,6 +17,7 @@ use crate::io::io_interface; use crate::package::PackageInterfaceHandle; use crate::repository::InstalledRepositoryInterface; use crate::repository::InstalledRepositoryInterfaceHandle; +use crate::signal::SignalSubscription; use crate::util::Platform; use crate::util::r#loop::Loop; use crate::util::sync_executor; @@ -25,7 +26,6 @@ use shirabe_php_shim::{ InvalidArgumentException, PhpMixed, array_splice, array_unshift, http_build_query, json_encode, str_contains, str_replace, strpos, strtolower, }; -use shirabe_seld_signal::SignalHandler; /// Package operation manager. #[derive(Debug)] @@ -309,19 +309,7 @@ impl InstallationManager { >, > = IndexMap::new(); - let signal_handler = SignalHandler::create( - vec![ - SignalHandler::SIGINT.to_string(), - SignalHandler::SIGTERM.to_string(), - SignalHandler::SIGHUP.to_string(), - ], - // TODO(phase-c): closure captures &mut self via &mut cleanup_promises - Box::new(move |signal: String, handler: &SignalHandler| { - // TODO(phase-c): self.io.write_error(...); self.run_cleanup(&cleanup_promises); - let _ = signal; - handler.exit_with_last_signal(); - }), - ); + let signals = SignalSubscription::new(); // Shared rather than owned so that one operation reaches a plugin as one object, both // through the whole batch pipeline and through its pre- and post-event. @@ -367,6 +355,9 @@ impl InstallationManager { } for batch_to_execute in batches { + if signals.is_triggered() { + sync_executor::block_on(self.abort_on_signal(&signals, &cleanup_promises)); + } sync_executor::block_on(self.download_and_execute_batch( repo, batch_to_execute, @@ -375,23 +366,26 @@ impl InstallationManager { run_scripts, download_only, all_operations.clone(), + &signals, ))?; } Ok(()) })(); - // finally - signal_handler.unregister(); - match result { Ok(()) => {} Err(e) => { + if signals.is_triggered() { + sync_executor::block_on(self.abort_on_signal(&signals, &cleanup_promises)); + } sync_executor::block_on(self.run_cleanup(&cleanup_promises)); return Err(e); } } + drop(signals); + if download_only { return Ok(()); } @@ -421,12 +415,16 @@ impl InstallationManager { run_scripts: bool, download_only: bool, all_operations: Vec<std::rc::Rc<AnyOperation>>, + signals: &SignalSubscription, ) -> anyhow::Result<()> { let mut promises: Vec< std::pin::Pin<Box<dyn std::future::Future<Output = anyhow::Result<()>>>>, > = vec![]; for (index, operation) in &operations { + if signals.is_triggered() { + self.abort_on_signal(signals, cleanup_promises).await; + } let op_type = operation.get_operation_type(); // ignoring alias ops as they don't need to execute anything at this stage @@ -496,6 +494,10 @@ impl InstallationManager { self.wait_on_promises(promises).await?; } + if signals.is_triggered() { + self.abort_on_signal(signals, cleanup_promises).await; + } + if download_only { self.run_cleanup(cleanup_promises).await; @@ -533,6 +535,9 @@ impl InstallationManager { } for batch_to_execute in batches { + if signals.is_triggered() { + self.abort_on_signal(signals, cleanup_promises).await; + } self.execute_batch( repo, batch_to_execute, @@ -540,6 +545,7 @@ impl InstallationManager { dev_mode, run_scripts, &all_operations, + signals, ) .await?; } @@ -547,6 +553,7 @@ impl InstallationManager { Ok(()) } + #[allow(clippy::too_many_arguments, reason = "to keep PHP signature")] async fn execute_batch( &self, repo: &InstalledRepositoryInterfaceHandle, @@ -562,6 +569,7 @@ impl InstallationManager { dev_mode: bool, run_scripts: bool, all_operations: &[std::rc::Rc<AnyOperation>], + signals: &SignalSubscription, ) -> anyhow::Result<()> { let mut promises: Vec< std::pin::Pin<Box<dyn std::future::Future<Output = anyhow::Result<()>> + '_>>, @@ -570,6 +578,9 @@ impl InstallationManager { let mut post_exec_callbacks: Vec<Box<dyn Fn() -> anyhow::Result<()>>> = vec![]; for (index, operation) in operations { + if signals.is_triggered() { + self.abort_on_signal(signals, cleanup_promises).await; + } let op_type = operation.get_operation_type(); // ignoring alias ops as they don't need to execute anything @@ -704,6 +715,10 @@ impl InstallationManager { self.wait_on_promises(promises).await?; } + if signals.is_triggered() { + self.abort_on_signal(signals, cleanup_promises).await; + } + Platform::workaround_filesystem_issues(); for cb in post_exec_callbacks { @@ -1022,6 +1037,28 @@ impl InstallationManager { let _ = self.loop_.borrow_mut().wait(promises, None).await; } } + + /// Cleans up after the packages installed so far and terminates the process. Never returns. + async fn abort_on_signal( + &self, + signals: &SignalSubscription, + cleanup_promises: &IndexMap< + i64, + Box< + dyn Fn() -> Option< + std::pin::Pin<Box<dyn std::future::Future<Output = anyhow::Result<()>>>>, + >, + >, + >, + ) { + self.io.write_error3( + &format!("Received {}, aborting", signals.last_signal().as_str()), + true, + io_interface::DEBUG, + ); + self.run_cleanup(cleanup_promises).await; + signals.exit_with_last_signal(); + } } // Composer's PartialComposer::setInstallationManager() accepts any InstallationManager subclass, so diff --git a/crates/shirabe/src/lib.rs b/crates/shirabe/src/lib.rs index 701a8dfd..009db5a0 100644 --- a/crates/shirabe/src/lib.rs +++ b/crates/shirabe/src/lib.rs @@ -22,6 +22,7 @@ pub mod question; pub mod repository; pub mod script; pub mod self_update; +pub mod signal; pub mod util; // InstalledVersions is intentionally unported to Rust. It is a runtime API for plugins and project diff --git a/crates/shirabe/src/signal.rs b/crates/shirabe/src/signal.rs new file mode 100644 index 00000000..a4765e59 --- /dev/null +++ b/crates/shirabe/src/signal.rs @@ -0,0 +1,205 @@ +//! Subscription to the signals that abort a Shirabe run: SIGINT, SIGTERM and SIGHUP. Plugins and +//! scripts cannot subscribe; `docs/dev/signals.md` says why. +//! +//! Composer reacts to these signals from `Seld\Signal\SignalHandler`'s pcntl callbacks, which the +//! interpreter runs inside whatever the process happens to be doing. A Rust signal handler may only +//! touch atomics, so the two halves are split here: the installed handler records the signal number, +//! the current subscription depth and a counter and then returns, and `is_triggered` reads those +//! atomics at checkpoints, so that the abort code runs on the normal call stack where it can borrow +//! whatever it needs. Checkpoints sit at the head of each loop that advances the work and on the +//! paths that handle an error, which is where an interrupted read surfaces. +//! +//! Subscriptions nest, and a signal belongs to the innermost one that was live when it arrived, and +//! to that one only. The depth recorded by the handler is what selects it; the counter is what +//! keeps a later subscription at the same depth from inheriting an older signal. Aborting from a +//! nested subscription therefore skips the clean-up of the ones around it — a `Ctrl+C` during a +//! child process spawned by `require` does not restore `composer.json`. `SignalHandler` dispatches +//! the same way, from one global stack to the most recently created handler, and undoing more than +//! Composer does would be an incompatibility of its own. +//! +//! Termination goes through `exit_with_last_signal`, which restores the default disposition and +//! re-raises the signal so the parent shell sees a signalled child rather than an ordinary exit. + +const HANDLED_SIGNALS: [nix::sys::signal::Signal; 3] = [ + nix::sys::signal::Signal::SIGINT, + nix::sys::signal::Signal::SIGTERM, + nix::sys::signal::Signal::SIGHUP, +]; + +/// Number of live `SignalSubscription`s. +static DEPTH: std::sync::atomic::AtomicUsize = std::sync::atomic::AtomicUsize::new(0); +/// Incremented once per delivered signal, so that a subscription can tell a signal that arrived +/// during its own lifetime from one that arrived before it was created. +static SIGNAL_SEQ: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0); +/// `DEPTH` as of the last delivered signal. +static SIGNALED_DEPTH: std::sync::atomic::AtomicUsize = std::sync::atomic::AtomicUsize::new(0); +static LAST_SIGNAL: std::sync::atomic::AtomicI32 = std::sync::atomic::AtomicI32::new(0); +static SAVED_SIGACTIONS: std::sync::Mutex< + Vec<(nix::sys::signal::Signal, nix::sys::signal::SigAction)>, +> = std::sync::Mutex::new(Vec::new()); + +extern "C" fn handle_signal(signal: nix::libc::c_int) { + LAST_SIGNAL.store(signal, std::sync::atomic::Ordering::SeqCst); + SIGNALED_DEPTH.store( + DEPTH.load(std::sync::atomic::Ordering::SeqCst), + std::sync::atomic::Ordering::SeqCst, + ); + SIGNAL_SEQ.fetch_add(1, std::sync::atomic::Ordering::SeqCst); +} + +/// Subscribes to SIGINT, SIGTERM and SIGHUP for as long as it is alive. Outside every subscription +/// the signals keep their default disposition and terminate the process outright. +#[derive(Debug)] +pub struct SignalSubscription { + depth: usize, + seq: u64, +} + +impl Default for SignalSubscription { + fn default() -> Self { + Self::new() + } +} + +impl SignalSubscription { + // TODO(phase-c): Windows delivers console control events (CTRL_C_EVENT, CTRL_BREAK_EVENT) + // rather than signals, and they are not subscribed to here. + pub fn new() -> Self { + let seq = SIGNAL_SEQ.load(std::sync::atomic::Ordering::SeqCst); + let mut saved_sigactions = SAVED_SIGACTIONS.lock().unwrap(); + let depth = DEPTH.fetch_add(1, std::sync::atomic::Ordering::SeqCst) + 1; + if depth == 1 { + // Without SA_RESTART a signal interrupts a blocking read instead of resuming it, which + // is what lets an interactive prompt inside a subscription reach the next checkpoint. + let action = nix::sys::signal::SigAction::new( + nix::sys::signal::SigHandler::Handler(handle_signal), + nix::sys::signal::SaFlags::empty(), + nix::sys::signal::SigSet::empty(), + ); + for signal in HANDLED_SIGNALS { + let previous = unsafe { nix::sys::signal::sigaction(signal, &action) } + .expect("failed to install a signal handler"); + saved_sigactions.push((signal, previous)); + } + } + Self { depth, seq } + } + + /// Whether a signal arrived while this subscription was the innermost one. + pub fn is_triggered(&self) -> bool { + SIGNAL_SEQ.load(std::sync::atomic::Ordering::SeqCst) > self.seq + && SIGNALED_DEPTH.load(std::sync::atomic::Ordering::SeqCst) == self.depth + } + + pub fn last_signal(&self) -> nix::sys::signal::Signal { + nix::sys::signal::Signal::try_from(LAST_SIGNAL.load(std::sync::atomic::Ordering::SeqCst)) + .expect("no signal has been delivered") + } + + /// Terminates the process the way the signal would have, so that the parent shell sees a + /// signalled child rather than an ordinary exit. + pub fn exit_with_last_signal(&self) -> ! { + let signal = self.last_signal(); + let default = nix::sys::signal::SigAction::new( + nix::sys::signal::SigHandler::SigDfl, + nix::sys::signal::SaFlags::empty(), + nix::sys::signal::SigSet::empty(), + ); + unsafe { nix::sys::signal::sigaction(signal, &default) } + .expect("failed to restore a signal handler"); + let _ = nix::sys::signal::raise(signal); + // usually the above raise() kills the process + // not strictly correct but it's the best we can do here + std::process::exit(128 + signal as i32); + } +} + +impl Drop for SignalSubscription { + fn drop(&mut self) { + let mut saved_sigactions = SAVED_SIGACTIONS.lock().unwrap(); + let depth = DEPTH.fetch_sub(1, std::sync::atomic::Ordering::SeqCst) - 1; + if depth == 0 { + for (signal, action) in saved_sigactions.drain(..) { + unsafe { nix::sys::signal::sigaction(signal, &action) } + .expect("failed to restore a signal handler"); + } + } + } +} + +#[cfg(test)] +mod tests { + use super::SignalSubscription; + + fn raise_sigint() { + nix::sys::signal::raise(nix::sys::signal::Signal::SIGINT).unwrap(); + } + + /// Runs this test binary again with `SIGNAL_TEST_CHILD` set, so that the signal may kill the + /// child instead of the test runner. + fn rerun_self(test_name: &str) -> std::process::ExitStatus { + std::process::Command::new(std::env::current_exe().unwrap()) + .arg(test_name) + .arg("--exact") + .env("SIGNAL_TEST_CHILD", "1") + .status() + .unwrap() + } + + fn terminating_signal(status: &std::process::ExitStatus) -> Option<i32> { + std::os::unix::process::ExitStatusExt::signal(status) + } + + #[test] + fn a_signal_belongs_to_the_innermost_subscription() { + let outer = SignalSubscription::new(); + { + let inner = SignalSubscription::new(); + assert!(!inner.is_triggered()); + raise_sigint(); + assert!(inner.is_triggered()); + assert_eq!(inner.last_signal(), nix::sys::signal::Signal::SIGINT); + assert!(!outer.is_triggered()); + } + { + // The signal above belongs to a subscription that is gone, not to this one. + assert!(!SignalSubscription::new().is_triggered()); + } + assert!(!outer.is_triggered()); + raise_sigint(); + assert!(outer.is_triggered()); + } + + #[test] + fn exit_with_last_signal_kills_by_the_signal() { + if std::env::var("SIGNAL_TEST_CHILD").is_ok() { + let signals = SignalSubscription::new(); + raise_sigint(); + signals.exit_with_last_signal(); + } + + let status = rerun_self("signal::tests::exit_with_last_signal_kills_by_the_signal"); + assert_eq!(status.code(), None); + assert_eq!( + terminating_signal(&status), + Some(nix::sys::signal::Signal::SIGINT as i32) + ); + } + + #[test] + fn the_default_disposition_returns_once_no_subscription_is_left() { + if std::env::var("SIGNAL_TEST_CHILD").is_ok() { + drop(SignalSubscription::new()); + raise_sigint(); + unreachable!("SIGINT must terminate the process"); + } + + let status = rerun_self( + "signal::tests::the_default_disposition_returns_once_no_subscription_is_left", + ); + assert_eq!( + terminating_signal(&status), + Some(nix::sys::signal::Signal::SIGINT as i32) + ); + } +} diff --git a/crates/shirabe/src/util/process_executor.rs b/crates/shirabe/src/util/process_executor.rs index 97f4ed3d..4808d0be 100644 --- a/crates/shirabe/src/util/process_executor.rs +++ b/crates/shirabe/src/util/process_executor.rs @@ -3,6 +3,7 @@ use crate::io::IOInterface; use crate::io::IOInterfaceImmutable; use crate::io::io_interface; +use crate::signal::SignalSubscription; use crate::util::GitHub; use crate::util::Platform; use indexmap::IndexMap; @@ -14,7 +15,6 @@ use shirabe_php_shim::{ php_regex, rtrim, str_replace, strcspn, strlen, strpbrk, strtolower, strtr_array, substr_replace, trim, }; -use shirabe_seld_signal::SignalHandler; use shirabe_symfony_process::ExecutableFinder; use shirabe_symfony_process::Process; use shirabe_symfony_process::ProcessMock; @@ -267,22 +267,7 @@ impl ProcessExecutor { // ignore TTY enabling errors } - let io_for_signal = self.io.clone(); - let signal_handler = SignalHandler::create( - vec![ - SignalHandler::SIGINT.to_string(), - SignalHandler::SIGTERM.to_string(), - SignalHandler::SIGHUP.to_string(), - ], - Box::new(move |signal: String, _h: &SignalHandler| { - if let Some(io) = &io_for_signal { - io.write_error(&format!( - "Received {}, aborting when child process is done", - signal - )); - } - }), - ); + let signals = SignalSubscription::new(); let result: anyhow::Result<()> = (|| -> anyhow::Result<()> { match output.to_callback() { @@ -306,23 +291,33 @@ impl ProcessExecutor { self.error_output = process.get_error_output()?; Ok(()) })(); + if signals.is_triggered() + && let Some(io) = &self.io + { + io.write_error3( + &format!( + "Received {}, aborting when child process is done", + signals.last_signal().as_str() + ), + true, + io_interface::DEBUG, + ); + } let final_result: anyhow::Result<()> = match result { Ok(()) => Ok(()), Err(e) => { if let Some(pse) = e.catch::<ProcessSignaledException>() { - if signal_handler.is_triggered() { + if signals.is_triggered() { // exiting as we were signaled and the child process exited too due to the signal - signal_handler.exit_with_last_signal(); + signals.exit_with_last_signal(); } let _ = pse; Ok(()) } else { - signal_handler.unregister(); return Err(e); } } }; - signal_handler.unregister(); final_result?; Ok(process.get_exit_code()) |