diff options
24 files changed, 401 insertions, 350 deletions
@@ -2090,6 +2090,7 @@ dependencies = [ "jsonschema", "md5", "mockall", + "nix", "regex", "reqwest", "serde", @@ -2103,7 +2104,6 @@ dependencies = [ "shirabe-php-rpc", "shirabe-php-shim", "shirabe-seld-json-lint", - "shirabe-seld-signal", "shirabe-semver", "shirabe-spdx-licenses", "shirabe-symfony-console", @@ -2202,10 +2202,6 @@ dependencies = [ ] [[package]] -name = "shirabe-seld-signal" -version = "0.0.1" - -[[package]] name = "shirabe-semver" version = "0.0.1" dependencies = [ @@ -19,7 +19,6 @@ shirabe-php-rpc = { path = "crates/shirabe-php-rpc" } shirabe-php-shim = { path = "crates/shirabe-php-shim" } shirabe-php-src = { path = "crates/shirabe-php-src" } shirabe-seld-json-lint = { path = "crates/shirabe-seld-json-lint" } -shirabe-seld-signal = { path = "crates/shirabe-seld-signal" } shirabe-semver = { path = "crates/shirabe-semver" } shirabe-spdx-licenses = { path = "crates/shirabe-spdx-licenses" } shirabe-symfony-console = { path = "crates/shirabe-symfony-console" } diff --git a/crates/shirabe-php-shim/src/process.rs b/crates/shirabe-php-shim/src/process.rs index b082f472..fd8b7d2d 100644 --- a/crates/shirabe-php-shim/src/process.rs +++ b/crates/shirabe-php-shim/src/process.rs @@ -396,22 +396,6 @@ pub fn getmypid() -> i64 { std::process::id() as i64 } -// No-op until real signal handling is wired up; signal registration itself is -// deferred (see the TODO(plugin) notes in SignalRegistry::register). -pub fn pcntl_async_signals(_enable: bool) {} - -pub fn pcntl_signal(_signal: i64, _handler: PhpMixed) -> bool { - // TODO(phase-c): registering a signal handler requires the signal-handling subsystem to be - // wired up (cf. SignalRegistry / the TODO(plugin) notes). sigaction(2) itself is reachable, but - // the handler is a PHP callable whose dispatch depends on the runtime callable mechanism. - todo!() -} - -pub fn pcntl_signal_get_handler(_signal: i64) -> PhpMixed { - // TODO(phase-c): see pcntl_signal; needs the signal-handling subsystem. - todo!() -} - pub fn posix_getuid() -> i64 { nix::unistd::getuid().as_raw() as i64 } diff --git a/crates/shirabe-php-shim/src/runtime.rs b/crates/shirabe-php-shim/src/runtime.rs index 2b5e7155..71356427 100644 --- a/crates/shirabe-php-shim/src/runtime.rs +++ b/crates/shirabe-php-shim/src/runtime.rs @@ -354,11 +354,6 @@ pub fn memory_get_peak_usage(_real_usage: bool) -> i64 { 0 } -pub fn call_php_callable(_callback: &PhpMixed, _args: &[PhpMixed]) -> PhpMixed { - // TODO(php-runtime): PhpMixed carries no callable variant; a runtime callable cannot be invoked. - todo!() -} - pub fn ini_set(_varname: &str, _value: &str) -> Option<String> { // TODO(php-runtime): ini_set must return the previous value and have its override observed by a // subsequent ini_get; ini_get is currently a static lookup, so overrides cannot be wired up yet. diff --git a/crates/shirabe-seld-signal/Cargo.toml b/crates/shirabe-seld-signal/Cargo.toml deleted file mode 100644 index 1dc7c3df..00000000 --- a/crates/shirabe-seld-signal/Cargo.toml +++ /dev/null @@ -1,11 +0,0 @@ -[package] -name = "shirabe-seld-signal" -version.workspace = true -edition.workspace = true -rust-version.workspace = true -description = "A Rust port of seld/signal-handler" -repository.workspace = true -license.workspace = true - -[lints] -workspace = true diff --git a/crates/shirabe-seld-signal/LICENSE b/crates/shirabe-seld-signal/LICENSE deleted file mode 100644 index c1b62a35..00000000 --- a/crates/shirabe-seld-signal/LICENSE +++ /dev/null @@ -1,19 +0,0 @@ -Copyright (c) 2015 Jordi Boggiano - -Permission is hereby granted, free of charge, to any person obtaining a copy -of this software and associated documentation files (the "Software"), to deal -in the Software without restriction, including without limitation the rights -to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -copies of the Software, and to permit persons to whom the Software is furnished -to do so, subject to the following conditions: - -The above copyright notice and this permission notice shall be included in all -copies or substantial portions of the Software. - -THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN -THE SOFTWARE. diff --git a/crates/shirabe-seld-signal/src/lib.rs b/crates/shirabe-seld-signal/src/lib.rs deleted file mode 100644 index 16f51510..00000000 --- a/crates/shirabe-seld-signal/src/lib.rs +++ /dev/null @@ -1,3 +0,0 @@ -pub mod signal_handler; - -pub use signal_handler::*; diff --git a/crates/shirabe-seld-signal/src/signal_handler.rs b/crates/shirabe-seld-signal/src/signal_handler.rs deleted file mode 100644 index b2f1873e..00000000 --- a/crates/shirabe-seld-signal/src/signal_handler.rs +++ /dev/null @@ -1,25 +0,0 @@ -//! ref: composer/vendor/seld/signal-handler/src/SignalHandler.php - -#[derive(Debug)] -pub struct SignalHandler; - -// TODO(phase-c): disable signal handler at all for now. -impl SignalHandler { - pub const SIGINT: &'static str = "SIGINT"; - pub const SIGTERM: &'static str = "SIGTERM"; - pub const SIGHUP: &'static str = "SIGHUP"; - - pub fn create(_signals: Vec<String>, _callback: Box<dyn Fn(String, &SignalHandler)>) -> Self { - Self - } - - pub fn unregister(&self) {} - - pub fn exit_with_last_signal(&self) { - std::process::exit(0); - } - - pub fn is_triggered(&self) -> bool { - false - } -} diff --git a/crates/shirabe-symfony-console/src/command.rs b/crates/shirabe-symfony-console/src/command.rs index 5ab97400..7efaf401 100644 --- a/crates/shirabe-symfony-console/src/command.rs +++ b/crates/shirabe-symfony-console/src/command.rs @@ -3,11 +3,9 @@ mod complete_command; mod dump_completion_command; mod help_command; mod list_command; -mod signalable_command_interface; pub use command::*; pub use complete_command::*; pub use dump_completion_command::*; pub use help_command::*; pub use list_command::*; -pub use signalable_command_interface::*; diff --git a/crates/shirabe-symfony-console/src/command/signalable_command_interface.rs b/crates/shirabe-symfony-console/src/command/signalable_command_interface.rs deleted file mode 100644 index 8777eb5e..00000000 --- a/crates/shirabe-symfony-console/src/command/signalable_command_interface.rs +++ /dev/null @@ -1,10 +0,0 @@ -//! ref: composer/vendor/symfony/console/Command/SignalableCommandInterface.php - -/// Interface for command reacting to signal. -pub trait SignalableCommandInterface { - /// Returns the list of signals to subscribe. - fn get_subscribed_signals(&self) -> Vec<i64>; - - /// The method will be called when the application is signaled. - fn handle_signal(&mut self, signal: i64); -} diff --git a/crates/shirabe-symfony-console/src/lib.rs b/crates/shirabe-symfony-console/src/lib.rs index 814e53a3..4a5a4ceb 100644 --- a/crates/shirabe-symfony-console/src/lib.rs +++ b/crates/shirabe-symfony-console/src/lib.rs @@ -12,7 +12,6 @@ pub mod helper; pub mod input; pub mod output; pub mod question; -pub mod signal_registry; pub mod style; pub mod terminal; pub mod tester; @@ -31,6 +30,5 @@ pub use helper::*; pub use input::*; pub use output::*; pub use question::*; -pub use signal_registry::*; pub use style::*; pub use terminal::*; diff --git a/crates/shirabe-symfony-console/src/signal_registry.rs b/crates/shirabe-symfony-console/src/signal_registry.rs deleted file mode 100644 index ab48848e..00000000 --- a/crates/shirabe-symfony-console/src/signal_registry.rs +++ /dev/null @@ -1,3 +0,0 @@ -mod signal_registry; - -pub use signal_registry::*; diff --git a/crates/shirabe-symfony-console/src/signal_registry/signal_registry.rs b/crates/shirabe-symfony-console/src/signal_registry/signal_registry.rs deleted file mode 100644 index f1f67d09..00000000 --- a/crates/shirabe-symfony-console/src/signal_registry/signal_registry.rs +++ /dev/null @@ -1,102 +0,0 @@ -//! ref: composer/vendor/symfony/console/SignalRegistry/SignalRegistry.php - -use indexmap::IndexMap; - -/// A signal handler receives the signal number and whether a further handler follows. -pub type SignalHandler = Box<dyn Fn(i64, bool)>; - -pub struct SignalRegistry { - // signal number => list of handlers - signal_handlers: IndexMap<i64, Vec<SignalHandler>>, -} - -impl std::fmt::Debug for SignalRegistry { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.debug_struct("SignalRegistry") - .field("signal_handlers", &self.signal_handlers.keys()) - .finish_non_exhaustive() - } -} - -impl Default for SignalRegistry { - fn default() -> Self { - Self::new() - } -} - -impl SignalRegistry { - pub fn new() -> Self { - if shirabe_php_shim::function_exists("pcntl_async_signals") { - shirabe_php_shim::pcntl_async_signals(true); - } - - Self { - signal_handlers: IndexMap::new(), - } - } - - pub fn register(&mut self, signal: i64, signal_handler: SignalHandler) { - if !self.signal_handlers.contains_key(&signal) { - let previous_callback = shirabe_php_shim::pcntl_signal_get_handler(signal); - - if shirabe_php_shim::is_callable(&previous_callback) { - // $this->signalHandlers[$signal][] = $previousCallback; - // The previous handler is an opaque PHP callable obtained from pcntl; - // it is invoked through the runtime callable mechanism. - self.signal_handlers - .entry(signal) - .or_default() - .push(Box::new(move |signal, has_next| { - shirabe_php_shim::call_php_callable( - &previous_callback, - &[ - shirabe_php_shim::PhpMixed::Int(signal), - shirabe_php_shim::PhpMixed::Bool(has_next), - ], - ); - })); - } - } - - self.signal_handlers - .entry(signal) - .or_default() - .push(signal_handler); - - // pcntl_signal($signal, [$this, 'handle']) - // TODO(plugin): the PHP callback `[$this, 'handle']` captures the registry - // instance. Wiring this object method as a C-level signal handler requires the - // runtime callable mechanism; see review notes. - shirabe_php_shim::pcntl_signal(signal, shirabe_php_shim::PhpMixed::Null); - } - - pub fn is_supported() -> bool { - if !shirabe_php_shim::function_exists("pcntl_signal") { - return false; - } - - if shirabe_php_shim::explode( - ",", - &shirabe_php_shim::ini_get("disable_functions").unwrap_or_default(), - ) - .contains(&"pcntl_signal".to_string()) - { - return false; - } - - true - } - - pub fn handle(&self, signal: i64) { - let handlers = match self.signal_handlers.get(&signal) { - Some(handlers) => handlers, - None => return, - }; - let count = handlers.len(); - - for (i, signal_handler) in handlers.iter().enumerate() { - let has_next = i != count - 1; - signal_handler(signal, has_next); - } - } -} diff --git a/crates/shirabe-symfony-process/src/pipes/abstract_pipes.rs b/crates/shirabe-symfony-process/src/pipes/abstract_pipes.rs index 8741926c..f7269eb3 100644 --- a/crates/shirabe-symfony-process/src/pipes/abstract_pipes.rs +++ b/crates/shirabe-symfony-process/src/pipes/abstract_pipes.rs @@ -44,6 +44,9 @@ impl AbstractPipes { } /// Returns true if a system call has been interrupted. + // TODO(php-runtime): `last_error` is never set. PHP fills it from a `set_error_handler` wrapped + // around `stream_select`, so this always reports false and an EINTR-interrupted `select` resets + // the pipes instead of being retried. pub(crate) fn has_system_call_been_interrupted(&mut self) -> bool { let last_error = self.last_error.take(); diff --git a/crates/shirabe/Cargo.toml b/crates/shirabe/Cargo.toml index 6d29c886..0790d937 100644 --- a/crates/shirabe/Cargo.toml +++ b/crates/shirabe/Cargo.toml @@ -15,7 +15,6 @@ shirabe-pcre.workspace = true shirabe-php-rpc.workspace = true shirabe-php-shim.workspace = true shirabe-seld-json-lint.workspace = true -shirabe-seld-signal.workspace = true shirabe-semver.workspace = true shirabe-spdx-licenses.workspace = true shirabe-symfony-console.workspace = true @@ -30,6 +29,7 @@ futures.workspace = true indexmap.workspace = true jsonschema.workspace = true md5.workspace = true +nix.workspace = true regex.workspace = true reqwest.workspace = true serde.workspace = true 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()) diff --git a/docs/dev/signals.md b/docs/dev/signals.md new file mode 100644 index 00000000..9616f0ee --- /dev/null +++ b/docs/dev/signals.md @@ -0,0 +1,46 @@ +# Signals + +## In Composer core + +Composer has three routes for handling signals: + +* `Seld\Signal\SignalHandler` +* `Symfony\Component\Console\SignalRegistry\SignalRegistry` and `SignalableCommandInterface` +* PHP builtins (`pcntl_signal()`, etc.) + +Of these, upstream Composer itself only ever uses `SignalHandler`, and Shirabe +behaves roughly the same way. The difference is when the interruption takes +effect, as described in [known incompatibilities](../known-incompatibilities.md). + +> Composer runs its abort handler almost immediately after the signal arrives. +> Shirabe, however, runs it at the next checkpoint instead, so stopping `shirabe` +> command by `Ctrl+C` may take more time than Composer. + +In PHP, the VM checks for a pending signal at the end of a loop and on a +function call, which gets the signal handler run almost immediately after the +signal arrives. Rust has no such mechanism, so we place the checkpoints by +hand; grep for `signals.is_triggered()` to find them. They are not as +fine-grained as a function call, so more work runs between receiving the signal +and starting the abort than Composer would let through, and it takes longer. + +## In plugins and scripts + +Shirabe treats signal handling in plugins and scripts as undefined behavior, +for two broad reasons. + +The first is that Shirabe consists of a Rust core process and a PHP worker +process that runs the plugins and scripts, which makes faithful reproduction +difficult. + +The second is that Composer itself does not fully account for plugins and +scripts subscribing to signals either. The `SignalHandler` that Composer uses +to handle signals overwrites an existing signal handler unconditionally. So +even when a plugin or script installs a handler through Symfony Console or a +PHP builtin, that handler is lost the moment execution reaches a place where +Composer uses `SignalHandler`. + +For these two reasons, Shirabe today neither restricts plugins and scripts from +installing signal handlers nor does anything special about it. What happens +when they do is not guaranteed. + +This stance may be withdrawn if a legitimate use case turns up. diff --git a/docs/known-incompatibilities.md b/docs/known-incompatibilities.md index daaa8bcd..97e4dad7 100644 --- a/docs/known-incompatibilities.md +++ b/docs/known-incompatibilities.md @@ -39,6 +39,16 @@ behavior is slightly different. See [docs/dev/xdebug.md](./dev/xdebug.md) for details. +## Signals + +Composer runs its abort handler almost immediately after the signal arrives. +Shirabe, however, runs it at the next checkpoint instead, so stopping `shirabe` +command by `Ctrl+C` may take more time than Composer. + +Signal handling in plugins and scripts is undefined behavior: it may or may not +work. See [docs/dev/signals.md](./dev/signals.md) for details. + + ## Plugins ### Reflection |