From f6afe70947aae33c589756aaf50f8066b6e5300c Mon Sep 17 00:00:00 2001 From: Andy Gayton Date: Thu, 4 Jul 2024 16:39:50 -0400 Subject: [PATCH] store ctrlc_handlers on PersistentPlugin --- .../nu-cmd-plugin/src/commands/plugin/add.rs | 4 +- crates/nu-parser/src/parse_keywords.rs | 2 +- crates/nu-plugin-engine/src/init.rs | 6 +- crates/nu-plugin-engine/src/persistent.rs | 59 +++++++++---------- crates/nu-protocol/src/engine/ctrlc.rs | 8 +++ 5 files changed, 46 insertions(+), 33 deletions(-) diff --git a/crates/nu-cmd-plugin/src/commands/plugin/add.rs b/crates/nu-cmd-plugin/src/commands/plugin/add.rs index 14f3541168..f1d5de6750 100644 --- a/crates/nu-cmd-plugin/src/commands/plugin/add.rs +++ b/crates/nu-cmd-plugin/src/commands/plugin/add.rs @@ -109,13 +109,15 @@ apparent the next time `nu` is next launched with that plugin registry file. let custom_path = call.get_flag(engine_state, stack, "plugin-config")?; // Start the plugin manually, to get the freshest signatures and to not affect engine - // state. Provide a GC config that will stop it ASAP + // state. Provide a GC config that will stop it ASAP. We don't pass ctrlc_handlers since + // this instance is temporary and won't be used to run commands. let plugin = Arc::new(PersistentPlugin::new( identity, PluginGcConfig { enabled: true, stop_after: 0, }, + None, )); let interface = plugin.clone().get_plugin(Some((engine_state, stack)))?; let metadata = interface.get_metadata()?; diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index ca7bbbfc2d..089c0b2fba 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -3838,7 +3838,7 @@ pub fn parse_register(working_set: &mut StateWorkingSet, lite_command: &LiteComm let metadata_and_signatures = plugin .clone() - .get(get_envs, None) + .get(get_envs) .and_then(|p| { let meta = p.get_metadata()?; let sigs = p.get_signature()?; diff --git a/crates/nu-plugin-engine/src/init.rs b/crates/nu-plugin-engine/src/init.rs index 44fa4f031f..7c2734314c 100644 --- a/crates/nu-plugin-engine/src/init.rs +++ b/crates/nu-plugin-engine/src/init.rs @@ -295,7 +295,11 @@ pub fn add_plugin_to_working_set( // Add it to / get it from the working set let plugin = working_set.find_or_create_plugin(identity, || { - Arc::new(PersistentPlugin::new(identity.clone(), gc_config.clone())) + Arc::new(PersistentPlugin::new( + identity.clone(), + gc_config.clone(), + working_set.permanent_state.ctrlc_handlers.clone(), + )) }); plugin.set_gc_config(&gc_config); diff --git a/crates/nu-plugin-engine/src/persistent.rs b/crates/nu-plugin-engine/src/persistent.rs index bd5fc77a8c..632d5fe177 100644 --- a/crates/nu-plugin-engine/src/persistent.rs +++ b/crates/nu-plugin-engine/src/persistent.rs @@ -37,6 +37,8 @@ struct MutableState { preferred_mode: Option, /// Garbage collector config gc_config: PluginGcConfig, + /// TODO: docs + ctrlc_handlers: Option, } #[derive(Debug, Clone, Copy)] @@ -58,7 +60,11 @@ struct RunningPlugin { impl PersistentPlugin { /// Create a new persistent plugin. The plugin will not be spawned immediately. - pub fn new(identity: PluginIdentity, gc_config: PluginGcConfig) -> PersistentPlugin { + pub fn new( + identity: PluginIdentity, + gc_config: PluginGcConfig, + ctrlc_handlers: Option, + ) -> PersistentPlugin { PersistentPlugin { identity, mutable: Mutex::new(MutableState { @@ -66,6 +72,7 @@ impl PersistentPlugin { metadata: None, preferred_mode: None, gc_config, + ctrlc_handlers, }), } } @@ -77,7 +84,6 @@ impl PersistentPlugin { pub fn get( self: Arc, envs: impl FnOnce() -> Result, ShellError>, - ctrlc_handlers: Option, ) -> Result { let mut mutable = self.mutable.lock().map_err(|_| ShellError::NushellFailed { msg: format!( @@ -99,9 +105,7 @@ impl PersistentPlugin { // TODO: We should probably store the envs somewhere, in case we have to launch without // envs (e.g. from a custom value) let envs = envs()?; - let result = self - .clone() - .spawn(&envs, &mut mutable, ctrlc_handlers.clone()); + let result = self.clone().spawn(&envs, &mut mutable); // Check if we were using an alternate communication mode and may need to fall back to // stdio. @@ -116,7 +120,7 @@ impl PersistentPlugin { mutable.preferred_mode); // Reset to stdio and try again, but this time don't catch any error mutable.preferred_mode = Some(PreferredCommunicationMode::Stdio); - self.clone().spawn(&envs, &mut mutable, ctrlc_handlers)?; + self.clone().spawn(&envs, &mut mutable)?; } Ok(mutable @@ -135,7 +139,6 @@ impl PersistentPlugin { self: Arc, envs: &HashMap, mutable: &mut MutableState, - ctrlc_handlers: Option, ) -> Result<(), ShellError> { // Make sure `running` is set to None to begin if let Some(running) = mutable.running.take() { @@ -214,10 +217,12 @@ impl PersistentPlugin { gc.stop_tracking(); // Set the mode and try again mutable.preferred_mode = Some(PreferredCommunicationMode::LocalSocket); - return self.spawn(envs, mutable, ctrlc_handlers); + return self.spawn(envs, mutable); } - let guard = ctrlc_handlers + let guard = mutable + .ctrlc_handlers + .as_mut() .map(|ctrlc_handlers| { let interface = interface.clone(); ctrlc_handlers.register(Box::new(move || { @@ -229,7 +234,7 @@ impl PersistentPlugin { mutable.running = Some(RunningPlugin { interface, gc, - _ctrlc_guard: guard, + _ctrlc_guard: guard.clone(), }); Ok(()) } @@ -335,26 +340,20 @@ impl GetPlugin for PersistentPlugin { self: Arc, mut context: Option<(&EngineState, &mut Stack)>, ) -> Result { - let ctrlc_handlers = context - .as_ref() - .and_then(|(engine_state, _)| engine_state.ctrlc_handlers.clone()); - self.get( - || { - // Get envs from the context if provided. - let envs = context - .as_mut() - .map(|(engine_state, stack)| { - // We need the current environment variables for `python` based plugins. Or - // we'll likely have a problem when a plugin is implemented in a virtual Python - // environment. - let stack = &mut stack.start_capture(); - nu_engine::env::env_to_strings(engine_state, stack) - }) - .transpose()?; + self.get(|| { + // Get envs from the context if provided. + let envs = context + .as_mut() + .map(|(engine_state, stack)| { + // We need the current environment variables for `python` based plugins. Or + // we'll likely have a problem when a plugin is implemented in a virtual Python + // environment. + let stack = &mut stack.start_capture(); + nu_engine::env::env_to_strings(engine_state, stack) + }) + .transpose()?; - Ok(envs.unwrap_or_default()) - }, - ctrlc_handlers, - ) + Ok(envs.unwrap_or_default()) + }) } } diff --git a/crates/nu-protocol/src/engine/ctrlc.rs b/crates/nu-protocol/src/engine/ctrlc.rs index 876861a495..e9275f6029 100644 --- a/crates/nu-protocol/src/engine/ctrlc.rs +++ b/crates/nu-protocol/src/engine/ctrlc.rs @@ -11,6 +11,14 @@ pub struct Handlers { next_id: Arc, } +impl Debug for Handlers { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("Handlers") + .field("next_id", &self.next_id) + .finish() + } +} + #[derive(Clone)] pub struct Guard { id: usize,