sh: properly handle sigint in interactive mode

grisenti · grisenti · commit dc97de7096bc · 2025-04-02T10:48:38.000+02:00
diff --git a/sh/src/builtin/trap.rs b/sh/src/builtin/trap.rs
@@ -14,7 +14,7 @@ use crate::signals::Signal;
 use std::fmt::Display;
 use std::str::FromStr;
 
-#[derive(Clone)]
+#[derive(Clone, PartialEq, Eq)]
 pub enum TrapAction {
     Default,
     Ignore,
diff --git a/sh/src/cli/vi/mod.rs b/sh/src/cli/vi/mod.rs
@@ -821,6 +821,13 @@ impl ViEditor {
         }
         Ok(Action::None)
     }
+
+    pub fn reset_current_line(&mut self) {
+        self.edit_line.clear();
+        self.cursor.position = 0;
+        self.current_history_command = 0;
+        self.mode = EditorMode::Insert;
+    }
 }
 
 impl Default for ViEditor {
diff --git a/sh/src/main.rs b/sh/src/main.rs
@@ -11,13 +11,14 @@ use crate::cli::args::{parse_args, ExecutionMode};
 use crate::cli::terminal::is_attached_to_terminal;
 use crate::cli::{clear_line, set_cursor_pos};
 use crate::shell::Shell;
-use crate::signals::setup_signal_handling;
+use crate::signals::{
+    handle_signal_ignore, handle_signal_write_to_signal_buffer, setup_signal_handling, Signal,
+};
 use crate::utils::is_process_in_foreground;
 use cli::terminal::read_nonblocking_char;
 use cli::vi::{Action, ViEditor};
 use gettextrs::{bind_textdomain_codeset, setlocale, textdomain, LocaleCategory};
-use nix::sys::signal::{sigaction, SaFlags, SigAction, SigSet};
-use nix::sys::signal::{SigHandler, Signal as NixSignal};
+use nix::libc;
 use std::error::Error;
 use std::io;
 use std::io::Write;
@@ -140,7 +141,14 @@ fn standard_repl(shell: &mut Shell) {
             flush_stdout();
         }
         std::thread::sleep(Duration::from_millis(16));
+        shell.signal_manager.reset_sigint_count();
         shell.handle_async_events();
+        if shell.signal_manager.get_sigint_count() > 0 {
+            program_buffer.clear();
+            line_buffer.clear();
+            println!();
+            eprint!("{}", shell.get_ps1());
+        }
         if shell.set_options.vi {
             return;
         }
@@ -149,7 +157,7 @@ fn standard_repl(shell: &mut Shell) {
 
 fn vi_repl(shell: &mut Shell) {
     let mut editor = ViEditor::default();
-    let mut buffer = Vec::new();
+    let mut program_buffer = Vec::new();
     let mut print_ps2 = false;
     clear_line();
     flush_stdout();
@@ -158,29 +166,29 @@ fn vi_repl(shell: &mut Shell) {
         while let Some(c) = read_nonblocking_char() {
             match editor.process_new_input(c, shell) {
                 Ok(Action::Execute(command)) => {
-                    buffer.extend(command.iter());
-                    if buffer.ends_with(b"\\\n") {
+                    program_buffer.extend(command.iter());
+                    if program_buffer.ends_with(b"\\\n") {
                         continue;
                     }
-                    let program_string = match std::str::from_utf8(&buffer) {
+                    let program_string = match std::str::from_utf8(&program_buffer) {
                         Ok(buf) => buf,
                         Err(_) => {
                             eprintln!("sh: invalid utf-8 sequence");
-                            buffer.clear();
+                            program_buffer.clear();
                             continue;
                         }
                     };
                     println!();
                     shell.terminal.reset();
                     match shell.execute_program(program_string) {
                         Ok(_) => {
-                            buffer.clear();
+                            program_buffer.clear();
                             print_ps2 = false;
                         }
                         Err(syntax_err) => {
                             if !syntax_err.could_be_resolved_with_more_input {
                                 eprintln!("sh: syntax error: {}", syntax_err.message);
-                                buffer.clear();
+                                program_buffer.clear();
                             } else {
                                 print_ps2 = true;
                             }
@@ -205,7 +213,14 @@ fn vi_repl(shell: &mut Shell) {
             flush_stdout()
         }
         std::thread::sleep(Duration::from_millis(16));
+        shell.signal_manager.reset_sigint_count();
         shell.handle_async_events();
+        if shell.signal_manager.get_sigint_count() > 0 {
+            program_buffer.clear();
+            editor.reset_current_line();
+            println!();
+            eprint!("{}", shell.get_ps1());
+        }
         if !shell.set_options.vi {
             return;
         }
@@ -218,24 +233,14 @@ fn interactive_shell(shell: &mut Shell) {
         nix::unistd::tcsetpgrp(io::stdin().as_fd(), pgid).unwrap();
     }
     shell.terminal.set_nonblocking_no_echo();
-    let ignore_action = SigAction::new(SigHandler::SigIgn, SaFlags::empty(), SigSet::empty());
-    unsafe {
-        sigaction(NixSignal::SIGQUIT, &ignore_action).unwrap();
-    }
-    unsafe {
-        sigaction(NixSignal::SIGTERM, &ignore_action).unwrap();
-    }
+    unsafe { handle_signal_ignore(Signal::SigQuit) }
+    unsafe { handle_signal_ignore(Signal::SigTerm) }
+    unsafe { handle_signal_write_to_signal_buffer(Signal::SigInt) }
     if shell.set_options.monitor {
         // job control signals
-        unsafe {
-            sigaction(NixSignal::SIGTTIN, &ignore_action).unwrap();
-        }
-        unsafe {
-            sigaction(NixSignal::SIGTTOU, &ignore_action).unwrap();
-        }
-        unsafe {
-            sigaction(NixSignal::SIGTSTP, &ignore_action).unwrap();
-        }
+        unsafe { handle_signal_ignore(Signal::SigTtin) }
+        unsafe { handle_signal_ignore(Signal::SigTtou) }
+        unsafe { handle_signal_ignore(Signal::SigTstp) }
     }
     loop {
         if shell.set_options.vi {
diff --git a/sh/src/shell/mod.rs b/sh/src/shell/mod.rs
@@ -264,8 +264,8 @@ impl Shell {
     }
 
     pub fn process_signals(&mut self) {
-        while let Some(action) = self.signal_manager.get_pending_action() {
-            self.execute_action(action.clone())
+        while let Some(action) = self.signal_manager.get_pending_action().cloned() {
+            self.execute_action(action)
         }
     }
 
@@ -971,6 +971,7 @@ impl Shell {
             history,
             set_options,
             is_interactive,
+            signal_manager: SignalManager::new(is_interactive),
             ..Default::default()
         }
     }
@@ -1032,7 +1033,7 @@ impl Default for Shell {
             is_interactive: false,
             last_lineno: 0,
             exit_action: TrapAction::Default,
-            signal_manager: SignalManager::default(),
+            signal_manager: SignalManager::new(false),
             background_jobs: JobManager::default(),
             history: History::new(32767),
             umask: !0o022 & 0o777,
diff --git a/sh/src/signals.rs b/sh/src/signals.rs
@@ -260,7 +260,7 @@ pub const SIGNALS: &[Signal] = &[
 static mut SIGNAL_WRITE: Option<RawFd> = None;
 static mut SIGNAL_READ: Option<RawFd> = None;
 
-extern "C" fn handle_signals(signal: libc::c_int) {
+extern "C" fn write_signal_to_buffer(signal: libc::c_int) {
     // SIGNAL_WRITE is never modified after the initial
     // setup, and is a valid file descriptor, so this is safe
     let fd = unsafe { BorrowedFd::borrow_raw(SIGNAL_WRITE.unwrap()) };
@@ -304,12 +304,50 @@ fn get_pending_signal() -> Option<Signal> {
     }
 }
 
+pub unsafe fn handle_signal_ignore(signal: Signal) {
+    sigaction(
+        signal.into(),
+        &SigAction::new(SigHandler::SigIgn, SaFlags::empty(), SigSet::empty()),
+    )
+    .unwrap();
+}
+
+pub unsafe fn handle_signal_default(signal: Signal) {
+    sigaction(
+        signal.into(),
+        &SigAction::new(SigHandler::SigDfl, SaFlags::empty(), SigSet::empty()),
+    )
+    .unwrap();
+}
+
+pub unsafe fn handle_signal_write_to_signal_buffer(signal: Signal) {
+    sigaction(
+        signal.into(),
+        &SigAction::new(
+            SigHandler::Handler(write_signal_to_buffer),
+            SaFlags::empty(),
+            SigSet::empty(),
+        ),
+    )
+    .unwrap();
+}
+
 #[derive(Clone)]
 pub struct SignalManager {
     actions: [TrapAction; Signal::Count as usize],
+    is_interactive: bool,
+    sigint_count: u32,
 }
 
 impl SignalManager {
+    pub fn new(is_interactive: bool) -> Self {
+        Self {
+            actions: [const { TrapAction::Default }; Signal::Count as usize],
+            is_interactive,
+            sigint_count: 0,
+        }
+    }
+
     pub fn reset(&mut self) {
         for signal in SIGNALS {
             let signal = *signal;
@@ -336,44 +374,35 @@ impl SignalManager {
 
     pub fn set_action(&mut self, signal: Signal, action: TrapAction) {
         assert!(signal != Signal::SigKill && signal != Signal::SigStop);
+
+        if self.is_interactive
+            && signal == Signal::SigInt
+            && (action == TrapAction::Ignore || action == TrapAction::Default)
+        {
+            // in interactive mode we always want catch sigint
+            unsafe { handle_signal_write_to_signal_buffer(Signal::SigInt) };
+            self.actions[signal as usize] = action;
+            return;
+        }
         match action {
             TrapAction::Default => {
-                unsafe {
-                    sigaction(
-                        signal.into(),
-                        &SigAction::new(SigHandler::SigDfl, SaFlags::empty(), SigSet::empty()),
-                    )
-                    .unwrap()
-                };
+                unsafe { handle_signal_default(signal) };
             }
             TrapAction::Ignore => {
-                unsafe {
-                    sigaction(
-                        signal.into(),
-                        &SigAction::new(SigHandler::SigIgn, SaFlags::empty(), SigSet::empty()),
-                    )
-                    .unwrap()
-                };
+                unsafe { handle_signal_ignore(signal) };
             }
             TrapAction::Commands(_) => {
-                unsafe {
-                    sigaction(
-                        signal.into(),
-                        &SigAction::new(
-                            SigHandler::Handler(handle_signals),
-                            SaFlags::empty(),
-                            SigSet::empty(),
-                        ),
-                    )
-                    .unwrap()
-                };
+                unsafe { handle_signal_write_to_signal_buffer(signal) };
             }
         }
         self.actions[signal as usize] = action;
     }
 
-    pub fn get_pending_action(&self) -> Option<&TrapAction> {
+    pub fn get_pending_action(&mut self) -> Option<&TrapAction> {
         if let Some(signal) = get_pending_signal() {
+            if signal == Signal::SigInt {
+                self.sigint_count += 1;
+            }
             Some(&self.actions[signal as usize])
         } else {
             None
@@ -385,12 +414,12 @@ impl SignalManager {
             .iter()
             .map(move |&signal| (signal, &self.actions[signal as usize]))
     }
-}
 
-impl Default for SignalManager {
-    fn default() -> Self {
-        Self {
-            actions: [const { TrapAction::Default }; Signal::Count as usize],
-        }
+    pub fn reset_sigint_count(&mut self) {
+        self.sigint_count = 0;
+    }
+
+    pub fn get_sigint_count(&self) -> u32 {
+        self.sigint_count
     }
 }

Original file line number	Diff line number	Diff line change
`@@ -821,6 +821,13 @@ impl ViEditor {`
`821`	`821`	`}`
`822`	`822`	`Ok(Action::None)`
`823`	`823`	`}`
	`824`	`+`
	`825`	`+ pub fn reset_current_line(&mut self) {`
	`826`	`+ self.edit_line.clear();`
	`827`	`+ self.cursor.position = 0;`
	`828`	`+ self.current_history_command = 0;`
	`829`	`+ self.mode = EditorMode::Insert;`
	`830`	`+ }`
`824`	`831`	`}`
`825`	`832`
`826`	`833`	`impl Default for ViEditor {`
Original file line number	Diff line number	Diff line change
`@@ -264,8 +264,8 @@ impl Shell {`
`264`	`264`	`}`
`265`	`265`
`266`	`266`	`pub fn process_signals(&mut self) {`
`267`		`- while let Some(action) = self.signal_manager.get_pending_action() {`
`268`		`- self.execute_action(action.clone())`
	`267`	`+ while let Some(action) = self.signal_manager.get_pending_action().cloned() {`
	`268`	`+ self.execute_action(action)`
`269`	`269`	`}`
`270`	`270`	`}`
`271`	`271`
`@@ -971,6 +971,7 @@ impl Shell {`
`971`	`971`	`history,`
`972`	`972`	`set_options,`
`973`	`973`	`is_interactive,`
	`974`	`+ signal_manager: SignalManager::new(is_interactive),`
`974`	`975`	`..Default::default()`
`975`	`976`	`}`
`976`	`977`	`}`
`@@ -1032,7 +1033,7 @@ impl Default for Shell {`
`1032`	`1033`	`is_interactive: false,`
`1033`	`1034`	`last_lineno: 0,`
`1034`	`1035`	`exit_action: TrapAction::Default,`
`1035`		`- signal_manager: SignalManager::default(),`
	`1036`	`+ signal_manager: SignalManager::new(false),`
`1036`	`1037`	`background_jobs: JobManager::default(),`
`1037`	`1038`	`history: History::new(32767),`
`1038`	`1039`	`umask: !0o022 & 0o777,`