diff options
author | Rory Dudley | 2024-02-27 03:49:01 -0700 |
---|---|---|
committer | Rory Dudley | 2024-02-27 03:49:01 -0700 |
commit | e94d09da9449cabd7ece2acd98d52b1946a922a4 (patch) | |
tree | f0fb980662c01bf0c37bc2cfe30dcc5f7952f869 /src | |
parent | 0548e74cb3227716cf445f27bd64b8c0b4d0f981 (diff) | |
download | dwarvish-e94d09da9449cabd7ece2acd98d52b1946a922a4.tar.gz |
Remove custom errors and fix background forking
Removes the custom errors in src/recite/erro.rs, and replaces them with
std::io::Errors throughout (recite(), incant_, macros).
Fixed a bug with the way forking to the background is handled, where
registering the signal handler in main for all processes would break
couplets (i.e. pipes). Instead, this sets up a new signal handler each
time a process is forked into the background. It uses a Vec<i32> to keep
track of all the background processes.
Notes
Notes:
First off, there is some defunct code in the main repl loop, which is an
example of killing zombie processes after each prompt. This should be
removed, but I kept it in, just in case I go back to it for some reason.
To be honest, I have no clue why this code works. In theory, I should
have to remove the pid from the pids: Vec<i32> if waitpid returns a
positive integer. However, when I tried this, it completely broke the
program. ¯\_(ツ)_/¯
Also, it's worth noting that registering a signal handler with
signal_hook::low_level::register, is somewhat costly, according to their
docs. Given that this only occurs for background processes that are
forked, however, I think it is acceptable.
Finally, we never unregister the signal handler, so I'm not sure if
that's still hanging out in memory somewhere or no.
Diffstat (limited to 'src')
-rw-r--r-- | src/main.rs | 23 | ||||
-rw-r--r-- | src/recite.rs | 60 | ||||
-rw-r--r-- | src/recite/erro.rs | 15 | ||||
-rw-r--r-- | src/recite/ps.rs | 44 |
4 files changed, 59 insertions, 83 deletions
diff --git a/src/main.rs b/src/main.rs index 44f2726..b87089a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -1,5 +1,4 @@ mod recite; -use libc::{waitpid, WNOHANG}; use recite::path::prefresh; use recite::Poem; use std::io::{self, Write}; @@ -43,6 +42,16 @@ fn repl(path: &Vec<&Path>, prompt: &str) { break; } + // Kill zombies + // TODO: Remove this (since it was implemented in incant_quiet()) + // unsafe { + // let mut status = -1; + // let mut pid = waitpid(-1, &mut status, WNOHANG); + // while pid > 0 { + // pid = waitpid(-1, &mut status, WNOHANG); + // } + // } + // Trim the input let poetry = String::from(poetry.trim()); @@ -56,7 +65,7 @@ fn repl(path: &Vec<&Path>, prompt: &str) { match poem { Some(poem) => match poem.recite(path, &mut bins) { Ok(_) => {} - Err(e) => eprintln!("dwvsh: {}", e), + Err(e) => eprintln!("dwvsh: {}", e.to_string().to_lowercase()), }, None => {} }; @@ -89,16 +98,6 @@ fn main() { io::stdout().flush().unwrap(); }) .unwrap(); - - signal_hook::low_level::register(signal_hook::consts::SIGCHLD, move || { - let mut status: i32 = -1; - let pid: i32 = waitpid(-1, &mut status, WNOHANG); - if pid != -1 { - print!("[&] + done {}", pid); - io::stdout().flush().unwrap(); - } - }) - .unwrap(); }; // Begin evaluating commands diff --git a/src/recite.rs b/src/recite.rs index bfcd6b1..f21a905 100644 --- a/src/recite.rs +++ b/src/recite.rs @@ -1,14 +1,13 @@ -pub mod erro; pub mod path; mod ps; -use crate::ctask; -use crate::task; +use crate::{ctask, task}; use core::fmt; -use erro::Mishap; +use libc::{waitpid, WNOHANG}; use path::prefresh; use std::io::{self, Write}; use std::path::Path; use std::process::{exit, Command, Stdio}; +use std::sync::{Arc, Mutex}; /// Describes the ending of a [Verse] /// @@ -50,29 +49,25 @@ impl fmt::Display for Meter { } impl Meter { - fn incant_none(verse: &Verse, out: &mut String) -> Result<i32, Mishap> { + fn incant_none(verse: &Verse, out: &mut String) -> Result<i32, io::Error> { let child = task!(verse, out); - let output = child - .wait_with_output() - .map_err(|_| Mishap::Terminated(verse.stanza.to_string()))?; + let output = child.wait_with_output()?; if !output.status.success() { - return Err(Mishap::Else(verse.stanza.to_string())); + return Ok(output.status.code().unwrap_or(-1)); } Ok(output.status.code().unwrap_or(0)) } - fn incant_couplet(verse: &Verse, out: &mut String) -> Result<i32, Mishap> { + fn incant_couplet(verse: &Verse, out: &mut String) -> Result<i32, io::Error> { let child = ctask!(verse, out); - let output = child - .wait_with_output() - .map_err(|_| Mishap::Terminated(verse.stanza.to_string()))?; + let output = child.wait_with_output()?; if !output.status.success() { - return Err(Mishap::Else(verse.stanza.to_string())); + return Ok(output.status.code().unwrap_or(-1)); } out.push_str( @@ -84,17 +79,41 @@ impl Meter { Ok(output.status.code().unwrap_or(0)) } - fn incant_quiet(verse: &Verse, out: &mut String) -> Result<i32, Mishap> { + fn incant_quiet( + verse: &Verse, + out: &mut String, + pids: &mut Arc<Mutex<Vec<i32>>>, + ) -> Result<i32, io::Error> { let child = task!(verse, out); println!("[&] {}", child.id()); + + pids.lock().unwrap().push(child.id() as i32); + let stanza = verse.stanza.to_string(); + let pids = Arc::clone(pids); + + unsafe { + signal_hook::low_level::register(signal_hook::consts::SIGCHLD, move || { + for pid in pids.lock().unwrap().iter() { + let mut pid = *pid; + let mut status: i32 = 0; + pid = waitpid(pid, &mut status, WNOHANG); + if pid > 0 { + print!("\n[&] + done {}", stanza); + io::stdout().flush().unwrap(); + } + } + }) + .unwrap(); + } + Ok(0) } - fn incant_and(verse: &Verse, out: &mut String) -> Result<i32, Mishap> { + fn incant_and(verse: &Verse, out: &mut String) -> Result<i32, io::Error> { Meter::incant_none(verse, out) } - fn incant_string(verse: &Verse, out: &mut String) -> Result<i32, Mishap> { + fn incant_string(verse: &Verse, out: &mut String) -> Result<i32, io::Error> { Meter::incant_none(verse, out) } } @@ -314,9 +333,10 @@ impl Poem { /// None => {} /// } /// ``` - pub fn recite(&self, path: &Vec<&Path>, bins: &mut Vec<String>) -> Result<(), Mishap> { + pub fn recite(&self, path: &Vec<&Path>, bins: &mut Vec<String>) -> Result<(), io::Error> { // Variable for storing the output of a piped verse let mut out: String = String::new(); + let mut pids: Arc<Mutex<Vec<i32>>> = Arc::new(Mutex::new(Vec::new())); // Loop through each verse in the poem for verse in self.verses.iter() { @@ -359,12 +379,12 @@ impl Poem { match verse.meter { Meter::None => Meter::incant_none(verse, &mut out)?, Meter::Couplet => Meter::incant_couplet(verse, &mut out)?, - Meter::Quiet => Meter::incant_quiet(verse, &mut out)?, + Meter::Quiet => Meter::incant_quiet(verse, &mut out, &mut pids)?, Meter::And => Meter::incant_and(verse, &mut out)?, Meter::String => match Meter::incant_string(verse, &mut out) { Ok(_) => 0, Err(e) => { - eprintln!("dwvsh: {}", e); + eprintln!("dwvsh: {}", e.to_string().to_lowercase()); 1 } }, diff --git a/src/recite/erro.rs b/src/recite/erro.rs deleted file mode 100644 index 00f57b3..0000000 --- a/src/recite/erro.rs +++ /dev/null @@ -1,15 +0,0 @@ -use thiserror::Error; - -#[derive(Error, Debug)] -pub enum Mishap { - #[error("broken pipe: {0}")] - BrokenPipe(String), - // #[error("exec format error: {0}")] - // ExecFormat(String), - #[error("permission denied: {0}")] - PermissionDenied(String), - #[error("terminated: {0}")] - Terminated(String), - #[error("exec error: {0}")] - Else(String), -} diff --git a/src/recite/ps.rs b/src/recite/ps.rs index 3557505..30c3d7c 100644 --- a/src/recite/ps.rs +++ b/src/recite/ps.rs @@ -5,30 +5,15 @@ macro_rules! task { let mut child = Command::new($verse.verb()) .args($verse.clause()) .stdin(Stdio::piped()) - .spawn() - .map_err(|e| match e.kind() { - io::ErrorKind::PermissionDenied => Mishap::PermissionDenied($verse.verb()), - _ => Mishap::Else($verse.verb()), - })?; + .spawn()?; - let stdin = child - .stdin - .as_mut() - .ok_or(Mishap::BrokenPipe($verse.verb()))?; - stdin - .write_all(&$out.as_bytes()) - .map_err(|_| Mishap::BrokenPipe($verse.verb()))?; + let stdin = child.stdin.as_mut().ok_or(io::ErrorKind::BrokenPipe)?; + stdin.write_all(&$out.as_bytes())?; $out.clear(); child } else { - Command::new($verse.verb()) - .args($verse.clause()) - .spawn() - .map_err(|e| match e.kind() { - io::ErrorKind::PermissionDenied => Mishap::PermissionDenied($verse.verb()), - _ => Mishap::Else($verse.verb()), - })? + Command::new($verse.verb()).args($verse.clause()).spawn()? } }; } @@ -41,19 +26,10 @@ macro_rules! ctask { .args($verse.clause()) .stdin(Stdio::piped()) .stdout(Stdio::piped()) - .spawn() - .map_err(|e| match e.kind() { - io::ErrorKind::PermissionDenied => Mishap::PermissionDenied($verse.verb()), - _ => Mishap::Else($verse.verb()), - })?; + .spawn()?; - let stdin = child - .stdin - .as_mut() - .ok_or(Mishap::BrokenPipe($verse.verb()))?; - stdin - .write_all(&$out.as_bytes()) - .map_err(|_| Mishap::BrokenPipe($verse.verb()))?; + let stdin = child.stdin.as_mut().ok_or(io::ErrorKind::BrokenPipe)?; + stdin.write_all(&$out.as_bytes())?; $out.clear(); child @@ -61,11 +37,7 @@ macro_rules! ctask { Command::new($verse.verb()) .args($verse.clause()) .stdout(Stdio::piped()) - .spawn() - .map_err(|e| match e.kind() { - io::ErrorKind::PermissionDenied => Mishap::PermissionDenied($verse.verb()), - _ => Mishap::Else($verse.verb()), - })? + .spawn()? } }; } |