-
Notifications
You must be signed in to change notification settings - Fork 15
feat: add job control and signal handling #89
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,98 @@ | ||
| use once_cell::sync::Lazy; | ||
| use std::process::Command; | ||
| use std::sync::atomic::{AtomicI32, Ordering}; | ||
| use std::thread; | ||
| use std::os::unix::process::CommandExt; | ||
| use libc; | ||
|
|
||
| static FG_PGID: Lazy<AtomicI32> = Lazy::new(|| AtomicI32::new(0)); | ||
|
|
||
| pub fn init_signal_handlers() { | ||
| static START: std::sync::Once = std::sync::Once::new(); | ||
| START.call_once(|| { | ||
| // Spawn a thread to handle signals using signal-hook's iterator | ||
| let _ = thread::spawn(|| { | ||
| let mut signals = match signal_hook::iterator::Signals::new(&[ | ||
| libc::SIGCHLD, | ||
| libc::SIGINT, | ||
| libc::SIGTSTP, | ||
| ]) { | ||
| Ok(s) => s, | ||
| Err(e) => { | ||
| eprintln!("Failed to register signal handlers: {}", e); | ||
| return; | ||
| } | ||
| }; | ||
|
|
||
| for signal in signals.forever() { | ||
| match signal { | ||
| libc::SIGCHLD => { | ||
| // Reap any finished children (non-blocking) | ||
| loop { | ||
| let mut status: libc::c_int = 0; | ||
| let pid = unsafe { libc::waitpid(-1, &mut status as *mut i32, libc::WNOHANG) }; | ||
| if pid <= 0 { | ||
| break; | ||
| } | ||
| } | ||
|
Comment on lines
+29
to
+37
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🔴 Critical | 🏗️ Heavy lift 🧩 Analysis chain🏁 Script executed: sed -n '1,140p' src/job_control.rs | nl -baRepository: TruFoundation/TruShell Length of output: 200 🏁 Script executed: sed -n '1,160p' src/job_control.rs | cat -nRepository: TruFoundation/TruShell Length of output: 4405 🏁 Script executed: rg -n "waitpid|WUNTRACED|SIGCHLD|SIGTSTP|try_wait|tcsetpgrp|setpgid" srcRepository: TruFoundation/TruShell Length of output: 1046 Use a single foreground child-status owner The SIGCHLD reaper can consume the foreground child’s status before the blocking 🤖 Prompt for AI Agents |
||
| } | ||
| libc::SIGINT => { | ||
| let pgid = FG_PGID.load(Ordering::SeqCst); | ||
| if pgid > 0 { | ||
| unsafe { libc::kill(-(pgid as libc::pid_t), libc::SIGINT) }; | ||
| } | ||
| } | ||
| libc::SIGTSTP => { | ||
| let pgid = FG_PGID.load(Ordering::SeqCst); | ||
| if pgid > 0 { | ||
| unsafe { libc::kill(-(pgid as libc::pid_t), libc::SIGTSTP) }; | ||
| } | ||
| } | ||
| _ => {} | ||
| } | ||
| } | ||
| }); | ||
| }); | ||
| } | ||
|
|
||
| pub fn spawn_and_wait(command_name: &str, args: &[String]) { | ||
| // Build the command and ensure the child is placed into its own process group | ||
| let mut cmd = Command::new(command_name); | ||
| cmd.args(args) | ||
| .stdin(std::process::Stdio::inherit()) | ||
| .stdout(std::process::Stdio::inherit()) | ||
| .stderr(std::process::Stdio::inherit()) | ||
| .before_exec(|| { | ||
| // create new process group for the child | ||
| let r = unsafe { libc::setpgid(0, 0) }; | ||
| if r != 0 { | ||
| return Err(std::io::Error::last_os_error()); | ||
| } | ||
| Ok(()) | ||
| }); | ||
|
|
||
| match cmd.spawn() { | ||
| Ok(mut child) => { | ||
| let pid = child.id() as i32; | ||
| FG_PGID.store(pid, Ordering::SeqCst); | ||
|
|
||
| // Give terminal control to child process group | ||
| unsafe { libc::tcsetpgrp(libc::STDIN_FILENO, pid) }; | ||
|
|
||
| // Wait for the child to exit (blocking) | ||
| let mut status: libc::c_int = 0; | ||
| let _ = unsafe { libc::waitpid(pid, &mut status as *mut i32, 0) }; | ||
|
|
||
| // Restore terminal control to the shell | ||
| let shell_pgid = unsafe { libc::getpgrp() }; | ||
| unsafe { libc::tcsetpgrp(libc::STDIN_FILENO, shell_pgid) }; | ||
| FG_PGID.store(0, Ordering::SeqCst); | ||
|
Comment on lines
+79
to
+89
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '\n== job_control.rs outline ==\n'
ast-grep outline src/job_control.rs --view expanded || true
printf '\n== tcsetpgrp / SIGTTOU references ==\n'
rg -n "tcsetpgrp|SIGTTOU|sigaction|signal|setpgid|getpgrp|FG_PGID" src Cargo.toml . -g '!target' || true
printf '\n== src/job_control.rs relevant lines ==\n'
nl -ba src/job_control.rs | sed -n '1,220p'Repository: TruFoundation/TruShell Length of output: 3521 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '\n== src/job_control.rs (first 220 lines) ==\n'
sed -n '1,220p' src/job_control.rs
printf '\n== src/main.rs ==\n'
sed -n '1,120p' src/main.rsRepository: TruFoundation/TruShell Length of output: 7516 Ignore 🤖 Prompt for AI Agents |
||
|
|
||
| // Attempt to reap the std::process::Child to avoid zombies | ||
| let _ = child.try_wait(); | ||
| } | ||
| Err(e) => { | ||
| eprintln!("trushell: command not found '{}': {}", command_name, e); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,11 +1,14 @@ | ||
| mod parser; | ||
| mod job_control; | ||
|
|
||
| use std::io::{self, Write}; | ||
| use std::process::{Command, Stdio}; | ||
|
|
||
| fn main() { | ||
| println!("Welcome to TruShell Native Engine"); | ||
|
|
||
| // Initialize job control and signal handlers | ||
| job_control::init_signal_handlers(); | ||
|
|
||
| loop { | ||
| print!("trushell ❯ "); | ||
| if let Err(e) = io::stdout().flush() { | ||
|
|
@@ -51,15 +54,15 @@ fn main() { | |
| // If the parsed AST looks like a CLI invocation that was | ||
| // accidentally parsed as subtraction (e.g. `ls -la` -> `ls - la`), | ||
| // fall back to executing the system command. | ||
| if let Some((cmd, args)) = probable_cli_from_ast(&ast) { | ||
| execute_system_command(&cmd, &args); | ||
| if let Some((cmd, args)) = probable_cli_from_ast(&ast) { | ||
| job_control::spawn_and_wait(&cmd, &args); | ||
| } else { | ||
| println!("Parsed AST: {:#?}", ast); | ||
| } | ||
| } | ||
| Err(err) => { | ||
| eprintln!("Parse error: {}", err); | ||
| execute_system_command_from_input(trimmed_input); | ||
| job_control::spawn_and_wait(&parts[0], &parts[1..].to_vec()); | ||
|
Comment on lines
63
to
+65
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '\n## src/main.rs (around lines 1-140)\n'
cat -n src/main.rs | sed -n '1,140p'
printf '\n## Search for split_posix_words and empty-token guards\n'
rg -n "split_posix_words|split_first\\(|parts\\[0\\]|spawn_and_wait" src -SRepository: TruFoundation/TruShell Length of output: 6371 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '\n## src/main.rs (split_posix_words and related fallback)\n'
cat -n src/main.rs | sed -n '120,240p'
printf '\n## parser.rs outline\n'
ast-grep outline src/parser.rs --view expanded || true
printf '\n## Search for empty-token handling around command execution\n'
rg -n "is_empty\\(|split_first\\(|spawn_and_wait\\(|parts\\[0\\]" src/main.rs src/job_control.rs src/parser.rs -SRepository: TruFoundation/TruShell Length of output: 7671 Preserve the empty-token guard in the parse-error fallback. 🧰 Tools🪛 Clippy (1.96.0)[warning] 65-65: unnecessary use of (warning) 🤖 Prompt for AI Agents |
||
| } | ||
| } | ||
| } | ||
|
|
@@ -173,23 +176,8 @@ fn execute_system_command(cmd: &str, args: &[String]) { | |
| cmd | ||
| }; | ||
|
|
||
| let child = Command::new(command_name) | ||
| .args(args) | ||
| .stdin(Stdio::inherit()) | ||
| .stdout(Stdio::inherit()) | ||
| .stderr(Stdio::inherit()) | ||
| .spawn(); | ||
|
|
||
| match child { | ||
| Ok(mut child_proc) => { | ||
| if let Err(e) = child_proc.wait() { | ||
| eprintln!("Execution error: {}", e); | ||
| } | ||
| } | ||
| Err(e) => { | ||
| eprintln!("trushell: command not found '{}': {}", command_name, e); | ||
| } | ||
| } | ||
| // Deprecated: routed to job_control::spawn_and_wait in main | ||
| job_control::spawn_and_wait(command_name, args); | ||
| } | ||
|
|
||
| fn execute_system_command_from_input(input: &str) { | ||
|
|
@@ -200,5 +188,5 @@ fn execute_system_command_from_input(input: &str) { | |
|
|
||
| let cmd = parts[0].clone(); | ||
| let args = parts.into_iter().skip(1).collect::<Vec<_>>(); | ||
| execute_system_command(&cmd, &args); | ||
| job_control::spawn_and_wait(&cmd, &args); | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
Repository: TruFoundation/TruShell
Length of output: 422
🏁 Script executed:
Repository: TruFoundation/TruShell
Length of output: 11436
🏁 Script executed:
Repository: TruFoundation/TruShell
Length of output: 4578
🌐 Web query:
signal-hook 0.3 iterator::Signals::new signature accepts slice or iterator💡 Result:
Yes, the signal-hook 0.3 library's Signals::new method accepts a slice iterator. Specifically, the method is defined with a generic signature that accepts any type I that implements IntoIterator where the items S can be borrowed as a c_int [1][2][3]. The signature is: pub fn new<I, S>(signals: I) -> Result<Self, Error> where I: IntoIterator<Item = S>, S: Borrow<c_int>, Because a reference to a slice, such as &[c_int], implements IntoIterator [1][3], you can pass a slice directly to Signals::new as shown in the following example: let mut signals = Signals::new(&[SIGUSR1, SIGUSR2])?; This flexibility allows you to pass arrays, vectors, or slices, as they all satisfy the IntoIterator trait requirement [2][3].
Citations:
Register the handlers before starting the worker thread.
Signals::newruns insidethread::spawn, soinit_signal_handlers()can return beforeSIGCHLD/SIGINT/SIGTSTPare registered. CreateSignalsfirst, then move it into the thread.🧰 Tools
🪛 Clippy (1.96.0)
[warning] 15-15: the borrowed expression implements the required traits
(warning)
🤖 Prompt for AI Agents