feat: make ctrlc available to plugins (#13181)

# Description

This PR adds a new method to `EngineInterface`: `register_ctrlc_handler`
which takes a closure to run when the plugin's driving engine receives a
ctrlc-signal. It also adds a mirror of the `signals` attribute from the
main shell `EngineState`.

This is an example of how a plugin which makes a long poll http request
can end the request on ctrlc:
https://github.com/cablehead/nu_plugin_http/blob/main/src/commands/request.rs#L68-L77

To facilitate the feature, a new attribute has been added to
`EngineState`: `ctrlc_handlers`. This is a Vec of closures that will be
run when the engine's process receives a ctrlc signal.

When plugins are added to an `engine_state` during a `merge_delta`, the
engine passes the ctrlc_handlers to the plugin's
`.configure_ctrlc_handler` method, which gives the plugin a chance to
register a handler that sends a ctrlc packet through the
`PluginInterface`, if an instance of the plugin is currently running.

On the plugin side: `EngineInterface` also has a ctrlc_handlers Vec of
closures. Plugin calls can use `register_ctrlc_handler` to register a
closure that will be called in the plugin process when the
PluginInput::Ctrlc command is received.

For future reference these are some alternate places that were
investigated for tying the ctrlc trigger to transmitting a Ctrlc packet
through the `PluginInterface`:

- Directly from `src/signals.rs`: the handler there would need a
reference to the Vec<Arc<RegisteredPlugins>>, which would require us to
wrap the plugins in a Mutex, which we don't want to do.

- have `PersistentPlugin.get_plugin` pass down the engine's
CtrlcHandlers to .get and then to .spawn (if the plugin isn't already
running). Once we have CtrlcHandlers in spawn, we can register a handler
to write directly to PluginInterface. We don't want to double down on
passing engine_state to spawn this way though, as it's unpredictable
because it would depend on whether the plugin has already been spawned
or not.

- pass `ctrlc_handlers` to PersistentPlugin::new so it can store it on
itself so it's available to spawn.

- in `PersistentPlugin.spawn`, create a handler that sends to a clone of
the GC event loop's tx. this has the same issues with regards to how to
get CtrlcHandlers to the spawn method, and is more complicated than a
handler that writes directly to PluginInterface

# User-Facing Changes

No breaking changes

---------

Co-authored-by: Ian Manske <ian.manske@pm.me>
This commit is contained in:
Andy Gayton
2024-07-30 09:29:18 -04:00
committed by GitHub
parent e3f78b8793
commit 7b82c6b482
17 changed files with 310 additions and 21 deletions

View File

@ -0,0 +1,139 @@
use std::fmt::Debug;
use std::sync::{Arc, Mutex};
use crate::{engine::Sequence, ShellError};
/// Handler is a closure that can be sent across threads and shared.
pub type Handler = Box<dyn Fn() + Send + Sync>;
/// Manages a collection of handlers.
#[derive(Clone)]
pub struct Handlers {
/// List of handler tuples containing an ID and the handler itself.
handlers: Arc<Mutex<Vec<(usize, Handler)>>>,
/// Sequence generator for unique IDs.
next_id: Arc<Sequence>,
}
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()
}
}
/// Guard that unregisters a handler when dropped.
#[derive(Clone)]
pub struct Guard {
/// Unique ID of the handler.
id: usize,
/// Reference to the handlers list.
handlers: Arc<Mutex<Vec<(usize, Handler)>>>,
}
impl Drop for Guard {
/// Drops the `Guard`, removing the associated handler from the list.
fn drop(&mut self) {
if let Ok(mut handlers) = self.handlers.lock() {
handlers.retain(|(id, _)| *id != self.id);
}
}
}
impl Debug for Guard {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.debug_struct("Guard").field("id", &self.id).finish()
}
}
impl Handlers {
pub fn new() -> Handlers {
let handlers = Arc::new(Mutex::new(vec![]));
let next_id = Arc::new(Sequence::default());
Handlers { handlers, next_id }
}
/// Registers a new handler and returns an RAII guard which will unregister the handler when
/// dropped.
pub fn register(&self, handler: Handler) -> Result<Guard, ShellError> {
let id = self.next_id.next()?;
if let Ok(mut handlers) = self.handlers.lock() {
handlers.push((id, handler));
}
Ok(Guard {
id,
handlers: Arc::clone(&self.handlers),
})
}
/// Runs all registered handlers.
pub fn run(&self) {
if let Ok(handlers) = self.handlers.lock() {
for (_, handler) in handlers.iter() {
handler();
}
}
}
}
impl Default for Handlers {
fn default() -> Self {
Self::new()
}
}
#[cfg(test)]
mod tests {
use super::*;
use std::sync::atomic::{AtomicBool, Ordering};
#[test]
/// Tests registering and running multiple handlers.
fn test_multiple_handlers() {
let handlers = Handlers::new();
let called1 = Arc::new(AtomicBool::new(false));
let called2 = Arc::new(AtomicBool::new(false));
let called1_clone = Arc::clone(&called1);
let called2_clone = Arc::clone(&called2);
let _guard1 = handlers.register(Box::new(move || {
called1_clone.store(true, Ordering::SeqCst);
}));
let _guard2 = handlers.register(Box::new(move || {
called2_clone.store(true, Ordering::SeqCst);
}));
handlers.run();
assert!(called1.load(Ordering::SeqCst));
assert!(called2.load(Ordering::SeqCst));
}
#[test]
/// Tests the dropping of a guard and ensuring the handler is unregistered.
fn test_guard_drop() {
let handlers = Handlers::new();
let called = Arc::new(AtomicBool::new(false));
let called_clone = Arc::clone(&called);
let guard = handlers.register(Box::new(move || {
called_clone.store(true, Ordering::Relaxed);
}));
// Ensure the handler is registered
assert_eq!(handlers.handlers.lock().unwrap().len(), 1);
drop(guard);
// Ensure the handler is removed after dropping the guard
assert_eq!(handlers.handlers.lock().unwrap().len(), 0);
handlers.run();
// Ensure the handler is not called after being dropped
assert!(!called.load(Ordering::Relaxed));
}
}

View File

@ -2,6 +2,7 @@ use crate::{
ast::Block,
debugger::{Debugger, NoopDebugger},
engine::{
ctrlc,
usage::{build_usage, Usage},
CachedFile, Command, CommandType, EnvVars, OverlayFrame, ScopeFrame, Stack, StateDelta,
Variable, Visibility, DEFAULT_OVERLAY_NAME,
@ -85,6 +86,7 @@ pub struct EngineState {
pub spans: Vec<Span>,
usage: Usage,
pub scope: ScopeFrame,
pub ctrlc_handlers: Option<ctrlc::Handlers>,
signals: Signals,
pub env_vars: Arc<EnvVars>,
pub previous_env_vars: Arc<HashMap<String, Value>>,
@ -145,6 +147,7 @@ impl EngineState {
0,
false,
),
ctrlc_handlers: None,
signals: Signals::empty(),
env_vars: Arc::new(
[(DEFAULT_OVERLAY_NAME.to_string(), HashMap::new())]
@ -268,8 +271,13 @@ impl EngineState {
#[cfg(feature = "plugin")]
if !delta.plugins.is_empty() {
// Replace plugins that overlap in identity.
for plugin in std::mem::take(&mut delta.plugins) {
// Connect plugins to the ctrlc handlers
if let Some(handlers) = &self.ctrlc_handlers {
plugin.clone().configure_ctrlc_handler(handlers)?;
}
// Replace plugins that overlap in identity.
if let Some(existing) = self
.plugins
.iter_mut()

View File

@ -9,6 +9,7 @@ mod engine_state;
mod error_handler;
mod overlay;
mod pattern_match;
mod sequence;
mod stack;
mod stack_out_dest;
mod state_delta;
@ -27,8 +28,11 @@ pub use engine_state::*;
pub use error_handler::*;
pub use overlay::*;
pub use pattern_match::*;
pub use sequence::*;
pub use stack::*;
pub use stack_out_dest::*;
pub use state_delta::*;
pub use state_working_set::*;
pub use variable::*;
pub mod ctrlc;

View File

@ -0,0 +1,64 @@
use crate::ShellError;
use std::sync::atomic::{AtomicUsize, Ordering::Relaxed};
/// Implements an atomically incrementing sequential series of numbers
#[derive(Debug, Default)]
pub struct Sequence(AtomicUsize);
impl Sequence {
/// Return the next available id from a sequence, returning an error on overflow
#[track_caller]
pub fn next(&self) -> Result<usize, ShellError> {
// It's totally safe to use Relaxed ordering here, as there aren't other memory operations
// that depend on this value having been set for safety
//
// We're only not using `fetch_add` so that we can check for overflow, as wrapping with the
// identifier would lead to a serious bug - however unlikely that is.
self.0
.fetch_update(Relaxed, Relaxed, |current| current.checked_add(1))
.map_err(|_| ShellError::NushellFailedHelp {
msg: "an accumulator for identifiers overflowed".into(),
help: format!("see {}", std::panic::Location::caller()),
})
}
}
#[test]
fn output_is_sequential() {
let sequence = Sequence::default();
for (expected, generated) in (0..1000).zip(std::iter::repeat_with(|| sequence.next())) {
assert_eq!(expected, generated.expect("error in sequence"));
}
}
#[test]
fn output_is_unique_even_under_contention() {
let sequence = Sequence::default();
std::thread::scope(|scope| {
// Spawn four threads, all advancing the sequence simultaneously
let threads = (0..4)
.map(|_| {
scope.spawn(|| {
(0..100000)
.map(|_| sequence.next())
.collect::<Result<Vec<_>, _>>()
})
})
.collect::<Vec<_>>();
// Collect all of the results into a single flat vec
let mut results = threads
.into_iter()
.flat_map(|thread| thread.join().expect("panicked").expect("error"))
.collect::<Vec<usize>>();
// Check uniqueness
results.sort();
let initial_length = results.len();
results.dedup();
let deduplicated_length = results.len();
assert_eq!(initial_length, deduplicated_length);
})
}

View File

@ -56,6 +56,13 @@ impl Signals {
}
}
/// Triggers an interrupt.
pub fn trigger(&self) {
if let Some(signals) = &self.signals {
signals.store(true, Ordering::Relaxed);
}
}
/// Returns whether an interrupt has been triggered.
#[inline]
pub fn interrupted(&self) -> bool {

View File

@ -1,6 +1,6 @@
use std::{any::Any, sync::Arc};
use crate::{PluginGcConfig, PluginIdentity, PluginMetadata, ShellError};
use crate::{engine::ctrlc, PluginGcConfig, PluginIdentity, PluginMetadata, ShellError};
/// Trait for plugins registered in the [`EngineState`](crate::engine::EngineState).
pub trait RegisteredPlugin: Send + Sync {
@ -34,4 +34,12 @@ pub trait RegisteredPlugin: Send + Sync {
/// This is necessary in order to allow `nu_plugin` to handle the implementation details of
/// plugins.
fn as_any(self: Arc<Self>) -> Arc<dyn Any + Send + Sync>;
/// Give this plugin a chance to register for Ctrl-C signals.
fn configure_ctrlc_handler(
self: Arc<Self>,
_handler: &ctrlc::Handlers,
) -> Result<(), ShellError> {
Ok(())
}
}