Fixed logic for EINTR propagation

This commit is contained in:
2026-01-28 20:48:29 -05:00
parent 7f3e1cfcee
commit 70f0e849ba
7 changed files with 121 additions and 34 deletions

View File

@@ -128,7 +128,7 @@ fn parse_job_id(arg: &str, blame: Span) -> ShResult<usize> {
} else { } else {
Err(ShErr::full( Err(ShErr::full(
ShErrKind::SyntaxErr, ShErrKind::SyntaxErr,
format!("Invalid fd arg: {}", arg), format!("Invalid arg: {}", arg),
blame, blame,
)) ))
} }

View File

@@ -164,10 +164,26 @@ impl JobTab {
&mut self.jobs &mut self.jobs
} }
pub fn curr_job(&self) -> Option<usize> { pub fn curr_job(&self) -> Option<usize> {
self.order.last().copied() // Find the most recent valid job (order can have stale entries)
for &id in self.order.iter().rev() {
if self.jobs.get(id).is_some_and(|slot| slot.is_some()) {
return Some(id);
}
}
None
} }
pub fn prev_job(&self) -> Option<usize> { pub fn prev_job(&self) -> Option<usize> {
self.order.last().copied() // Find the second most recent valid job
let mut found_curr = false;
for &id in self.order.iter().rev() {
if self.jobs.get(id).is_some_and(|slot| slot.is_some()) {
if found_curr {
return Some(id);
}
found_curr = true;
}
}
None
} }
pub fn close_job_fds(&mut self, pid: Pid) { pub fn close_job_fds(&mut self, pid: Pid) {
self.fd_registry.retain(|fd| fd.owner_pid != pid) self.fd_registry.retain(|fd| fd.owner_pid != pid)
@@ -515,15 +531,19 @@ impl Job {
stats.push(WtStat::Exited(child.pid, code)); stats.push(WtStat::Exited(child.pid, code));
continue; continue;
} }
loop {
let result = child.wait(Some(WtFlag::WSTOPPED)); let result = child.wait(Some(WtFlag::WSTOPPED));
match result { match result {
Ok(stat) => { Ok(stat) => {
stats.push(stat); stats.push(stat);
break;
} }
Err(Errno::ECHILD) => break, Err(Errno::ECHILD) => break,
Err(Errno::EINTR) => continue, // Retry on signal interruption
Err(e) => return Err(e.into()), Err(e) => return Err(e.into()),
} }
} }
}
Ok(stats) Ok(stats)
} }
pub fn update_by_id(&mut self, id: JobID, stat: WtStat) -> ShResult<()> { pub fn update_by_id(&mut self, id: JobID, stat: WtStat) -> ShResult<()> {
@@ -649,6 +669,7 @@ pub fn wait_fg(job: Job) -> ShResult<()> {
} }
flog!(TRACE, "Waiting on foreground job"); flog!(TRACE, "Waiting on foreground job");
let mut code = 0; let mut code = 0;
let mut was_stopped = false;
attach_tty(job.pgid())?; attach_tty(job.pgid())?;
disable_reaping(); disable_reaping();
let statuses = write_jobs(|j| j.new_fg(job))?; let statuses = write_jobs(|j| j.new_fg(job))?;
@@ -658,11 +679,13 @@ pub fn wait_fg(job: Job) -> ShResult<()> {
code = exit_code; code = exit_code;
} }
WtStat::Stopped(_, sig) => { WtStat::Stopped(_, sig) => {
was_stopped = true;
write_jobs(|j| j.fg_to_bg(status))?; write_jobs(|j| j.fg_to_bg(status))?;
code = SIG_EXIT_OFFSET + sig as i32; code = SIG_EXIT_OFFSET + sig as i32;
} }
WtStat::Signaled(_, sig, _) => { WtStat::Signaled(_, sig, _) => {
if sig == Signal::SIGTSTP { if sig == Signal::SIGTSTP {
was_stopped = true;
write_jobs(|j| j.fg_to_bg(status))?; write_jobs(|j| j.fg_to_bg(status))?;
} }
code = SIG_EXIT_OFFSET + sig as i32; code = SIG_EXIT_OFFSET + sig as i32;
@@ -670,6 +693,10 @@ pub fn wait_fg(job: Job) -> ShResult<()> {
_ => { /* Do nothing */ } _ => { /* Do nothing */ }
} }
} }
// If job wasn't stopped (moved to bg), clear the fg slot
if !was_stopped {
write_jobs(|j| { j.take_fg(); });
}
take_term()?; take_term()?;
set_status(code); set_status(code);
flog!(TRACE, "exit code: {}", code); flog!(TRACE, "exit code: {}", code);

View File

@@ -374,9 +374,9 @@ impl Display for ShErr {
} }
impl From<std::io::Error> for ShErr { impl From<std::io::Error> for ShErr {
fn from(_: std::io::Error) -> Self { fn from(e: std::io::Error) -> Self {
let msg = std::io::Error::last_os_error(); let msg = std::io::Error::last_os_error();
ShErr::simple(ShErrKind::IoErr, msg.to_string()) ShErr::simple(ShErrKind::IoErr(e.kind()), msg.to_string())
} }
} }
@@ -394,7 +394,7 @@ impl From<Errno> for ShErr {
#[derive(Debug, Clone)] #[derive(Debug, Clone)]
pub enum ShErrKind { pub enum ShErrKind {
IoErr, IoErr(io::ErrorKind),
SyntaxErr, SyntaxErr,
ParseErr, ParseErr,
InternalErr, InternalErr,
@@ -420,7 +420,7 @@ pub enum ShErrKind {
impl Display for ShErrKind { impl Display for ShErrKind {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let output = match self { let output = match self {
Self::IoErr => "I/O Error", Self::IoErr(e) => &format!("I/O Error: {e}"),
Self::SyntaxErr => "Syntax Error", Self::SyntaxErr => "Syntax Error",
Self::ParseErr => "Parse Error", Self::ParseErr => "Parse Error",
Self::InternalErr => "Internal Error", Self::InternalErr => "Internal Error",

View File

@@ -106,6 +106,17 @@ fn fern_interactive() {
let mut partial_input = String::new(); let mut partial_input = String::new();
'outer: loop { 'outer: loop {
while signals_pending() {
if let Err(e) = check_signals() {
if let ShErrKind::ClearReadline = e.kind() {
partial_input.clear();
if !signals_pending() {
continue 'outer;
}
};
eprintln!("{e}");
}
}
// Main loop // Main loop
let edit_mode = write_shopts(|opt| opt.query("prompt.edit_mode")) let edit_mode = write_shopts(|opt| opt.query("prompt.edit_mode"))
.unwrap() .unwrap()

View File

@@ -48,11 +48,22 @@ impl Readline for FernVi {
loop { loop {
raw_mode_guard.disable_for(|| self.print_line())?; raw_mode_guard.disable_for(|| self.print_line())?;
let Some(key) = self.reader.read_key()? else { let key = match self.reader.read_key() {
Ok(Some(key)) => key,
Err(e) if matches!(e.kind(), ShErrKind::IoErr(std::io::ErrorKind::Interrupted)) => {
flog!(DEBUG, "readline interrupted");
let partial: String = self.editor.as_str().to_string();
return Err(ShErr::simple(ShErrKind::ReadlineIntr(partial), ""));
}
Err(_) | Ok(None) => {
flog!(DEBUG, "EOF detected");
raw_mode_guard.disable_for(|| self.writer.flush_write("\n"))?; raw_mode_guard.disable_for(|| self.writer.flush_write("\n"))?;
std::mem::drop(raw_mode_guard); std::mem::drop(raw_mode_guard);
return Err(ShErr::simple(ShErrKind::ReadlineErr, "EOF")); return Err(ShErr::simple(ShErrKind::ReadlineErr, "EOF"));
}
}; };
flog!(DEBUG, key); flog!(DEBUG, key);
if self.should_accept_hint(&key) { if self.should_accept_hint(&key) {

View File

@@ -28,6 +28,8 @@ pub fn raw_mode() -> RawModeGuard {
.expect("Failed to get terminal attributes"); .expect("Failed to get terminal attributes");
let mut raw = orig.clone(); let mut raw = orig.clone();
termios::cfmakeraw(&mut raw); termios::cfmakeraw(&mut raw);
// Keep ISIG enabled so Ctrl+C/Ctrl+Z still generate signals
raw.local_flags |= termios::LocalFlags::ISIG;
termios::tcsetattr( termios::tcsetattr(
unsafe { BorrowedFd::borrow_raw(STDIN_FILENO) }, unsafe { BorrowedFd::borrow_raw(STDIN_FILENO) },
termios::SetArg::TCSANOW, termios::SetArg::TCSANOW,
@@ -230,9 +232,15 @@ impl TermBuffer {
impl Read for TermBuffer { impl Read for TermBuffer {
fn read(&mut self, buf: &mut [u8]) -> std::io::Result<usize> { fn read(&mut self, buf: &mut [u8]) -> std::io::Result<usize> {
assert!(isatty(self.tty).is_ok_and(|r| r)); assert!(isatty(self.tty).is_ok_and(|r| r));
match nix::unistd::read(self.tty, buf) { flog!(DEBUG, "TermBuffer::read() ENTERING read syscall");
let result = nix::unistd::read(self.tty, buf);
flog!(DEBUG, "TermBuffer::read() EXITED read syscall: {:?}", result);
match result {
Ok(n) => Ok(n), Ok(n) => Ok(n),
Err(Errno::EINTR) => Err(Errno::EINTR.into()), Err(Errno::EINTR) => {
flog!(DEBUG, "TermBuffer::read() returning EINTR");
Err(Errno::EINTR.into())
}
Err(e) => Err(std::io::Error::from_raw_os_error(e as i32)), Err(e) => Err(std::io::Error::from_raw_os_error(e as i32)),
} }
} }
@@ -258,6 +266,8 @@ impl RawModeGuard {
// Re-enable raw mode // Re-enable raw mode
let mut raw = self.orig.clone(); let mut raw = self.orig.clone();
termios::cfmakeraw(&mut raw); termios::cfmakeraw(&mut raw);
// Keep ISIG enabled so Ctrl+C/Ctrl+Z still generate signals
raw.local_flags |= termios::LocalFlags::ISIG;
termios::tcsetattr(fd, termios::SetArg::TCSANOW, &raw).expect("Failed to re-enable raw mode"); termios::tcsetattr(fd, termios::SetArg::TCSANOW, &raw).expect("Failed to re-enable raw mode");
result result
@@ -315,7 +325,7 @@ impl TermReader {
pub fn next_byte(&mut self) -> std::io::Result<u8> { pub fn next_byte(&mut self) -> std::io::Result<u8> {
let mut buf = [0u8]; let mut buf = [0u8];
self.buffer.read_exact(&mut buf)?; let _n = self.buffer.read(&mut buf)?;
Ok(buf[0]) Ok(buf[0])
} }

View File

@@ -29,29 +29,38 @@ pub fn signals_pending() -> bool {
pub fn check_signals() -> ShResult<()> { pub fn check_signals() -> ShResult<()> {
if GOT_SIGINT.swap(false, Ordering::SeqCst) { if GOT_SIGINT.swap(false, Ordering::SeqCst) {
flog!(DEBUG, "check_signals: processing SIGINT");
interrupt()?; interrupt()?;
return Err(ShErr::simple(ShErrKind::ClearReadline, "")); return Err(ShErr::simple(ShErrKind::ClearReadline, ""));
} }
if GOT_SIGHUP.swap(false, Ordering::SeqCst) { if GOT_SIGHUP.swap(false, Ordering::SeqCst) {
flog!(DEBUG, "check_signals: processing SIGHUP");
hang_up(0); hang_up(0);
} }
if GOT_SIGTSTP.swap(false, Ordering::SeqCst) { if GOT_SIGTSTP.swap(false, Ordering::SeqCst) {
flog!(DEBUG, "check_signals: processing SIGTSTP");
terminal_stop()?; terminal_stop()?;
} }
if REAPING_ENABLED.load(Ordering::SeqCst) && GOT_SIGCHLD.swap(false, Ordering::SeqCst) { if REAPING_ENABLED.load(Ordering::SeqCst) && GOT_SIGCHLD.swap(false, Ordering::SeqCst) {
flog!(DEBUG, "check_signals: processing SIGCHLD (reaping enabled)");
wait_child()?; wait_child()?;
} else if GOT_SIGCHLD.load(Ordering::SeqCst) {
flog!(DEBUG, "check_signals: SIGCHLD pending but reaping disabled");
} }
if SHOULD_QUIT.load(Ordering::SeqCst) { if SHOULD_QUIT.load(Ordering::SeqCst) {
let code = QUIT_CODE.load(Ordering::SeqCst); let code = QUIT_CODE.load(Ordering::SeqCst);
flog!(DEBUG, "check_signals: SHOULD_QUIT set, exiting with code {}", code);
return Err(ShErr::simple(ShErrKind::CleanExit(code), "exit")); return Err(ShErr::simple(ShErrKind::CleanExit(code), "exit"));
} }
Ok(()) Ok(())
} }
pub fn disable_reaping() { pub fn disable_reaping() {
flog!(DEBUG, "disable_reaping: turning off SIGCHLD processing");
REAPING_ENABLED.store(false, Ordering::SeqCst); REAPING_ENABLED.store(false, Ordering::SeqCst);
} }
pub fn enable_reaping() { pub fn enable_reaping() {
flog!(DEBUG, "enable_reaping: turning on SIGCHLD processing");
REAPING_ENABLED.store(true, Ordering::SeqCst); REAPING_ENABLED.store(true, Ordering::SeqCst);
} }
@@ -142,10 +151,13 @@ extern "C" fn handle_sigint(_: libc::c_int) {
} }
pub fn interrupt() -> ShResult<()> { pub fn interrupt() -> ShResult<()> {
flog!(DEBUG, "interrupt: checking for fg job to send SIGINT");
write_jobs(|j| { write_jobs(|j| {
if let Some(job) = j.get_fg_mut() { if let Some(job) = j.get_fg_mut() {
flog!(DEBUG, "interrupt: sending SIGINT to fg job pgid {}", job.pgid());
job.killpg(Signal::SIGINT) job.killpg(Signal::SIGINT)
} else { } else {
flog!(DEBUG, "interrupt: no fg job, clearing readline");
Ok(()) Ok(())
} }
}) })
@@ -161,14 +173,30 @@ extern "C" fn handle_sigchld(_: libc::c_int) {
} }
pub fn wait_child() -> ShResult<()> { pub fn wait_child() -> ShResult<()> {
flog!(DEBUG, "wait_child: starting reap loop");
let flags = WtFlag::WNOHANG | WtFlag::WSTOPPED; let flags = WtFlag::WNOHANG | WtFlag::WSTOPPED;
while let Ok(status) = waitpid(None, Some(flags)) { while let Ok(status) = waitpid(None, Some(flags)) {
match status { match status {
WtStat::Exited(pid, _) => child_exited(pid, status)?, WtStat::Exited(pid, code) => {
WtStat::Signaled(pid, signal, _) => child_signaled(pid, signal)?, flog!(DEBUG, "wait_child: pid {} exited with code {}", pid, code);
WtStat::Stopped(pid, signal) => child_stopped(pid, signal)?, child_exited(pid, status)?;
WtStat::Continued(pid) => child_continued(pid)?, }
WtStat::StillAlive => break, WtStat::Signaled(pid, signal, _) => {
flog!(DEBUG, "wait_child: pid {} signaled with {:?}", pid, signal);
child_signaled(pid, signal)?;
}
WtStat::Stopped(pid, signal) => {
flog!(DEBUG, "wait_child: pid {} stopped with {:?}", pid, signal);
child_stopped(pid, signal)?;
}
WtStat::Continued(pid) => {
flog!(DEBUG, "wait_child: pid {} continued", pid);
child_continued(pid)?;
}
WtStat::StillAlive => {
flog!(DEBUG, "wait_child: no more children to reap");
break;
}
_ => unimplemented!(), _ => unimplemented!(),
} }
} }