Job control: libc constants, pid fix (#362)

* Use signal constants from libc Instead of hardcoding numbers, which causes portability issues. In particular, SIGTSTP is not 20 on FreeBSD. * Remove pid offset
redox-os · Jul 2, 2017 · 8975f981a9c20437b6d26675d84f8794656476c5 · 8975f98
1 parent c38bf35
commit 8975f981a9c20437b6d26675d84f8794656476c5
Unified Split

Showing with 36 additions and 27 deletions.

+1 −1 README.md

+8 −5 src/builtins/job_control.rs

+2 −2 src/main.rs

+3 −1 src/shell/flow.rs

+13 −10 src/shell/job_control.rs

+9 −8 src/shell/pipe.rs
diff --git a/README.md b/README.md
@@ -77,7 +77,7 @@ Below is an overview of features that Ion has either already implemented, or aim
 - [x] Background Jobs
 - [ ] Piping Functions
 - [ ] Redirecting Functions
- [ ] Background Job Control
+- [x] Background Job Control
 - [ ] XDG App Dirs
 - [ ] Plugins Support
    - [ ] Builtins
diff --git a/src/builtins/job_control.rs b/src/builtins/job_control.rs
@@ -5,7 +5,7 @@ use std::io::{stderr, Write};
 use std::thread::sleep;
 use std::time::Duration;
 #[cfg(not(target_os = "redox"))] use nix::sys::signal::{self, Signal};
-#[cfg(not(target_os = "redox"))] use libc::pid_t;
+#[cfg(not(target_os = "redox"))] use libc::{self, pid_t};

 /// Display a list of all jobs running in the background.
 pub fn jobs(shell: &mut Shell) {
@@ -19,22 +19,25 @@ pub fn jobs(shell: &mut Shell) {
    }
 }

+#[cfg(not(target_os = "redox"))]
 fn fg_listen(shell: &mut Shell, job: u32) {
    loop {
        sleep(Duration::from_millis(100));
        let job = &mut (*shell.background.lock().unwrap())[job as usize];
        if let ProcessState::Empty = job.state { break }
        if let Ok(signal) = shell.signals.try_recv() {
+            let stderr = stderr();
+            let _ = writeln!(stderr.lock(), "ion: fg_listen: signal {} ", signal);
            match signal {
-                20 => {
+                libc::SIGTSTP => {
                    let _ = signal::kill(job.pid as pid_t, Some(Signal::SIGTSTP));
                    job.state = ProcessState::Stopped;
                    break
                },
-                15 => {
-                    shell.handle_signal(15);
+                libc::SIGTERM => {
+                    shell.handle_signal(libc::SIGTERM);
                },
-                2 => {
+                libc::SIGINT => {
                    let _ = signal::kill(job.pid as pid_t, Some(Signal::SIGINT));
                    job.state = ProcessState::Empty;
                    break
diff --git a/src/main.rs b/src/main.rs
@@ -73,10 +73,10 @@ fn main() {
    let mut core = Core::new().unwrap();
    let handle = core.handle();

-    // Create a stream that will select over SIGINT and SIGTERM signals.
+    // Create a stream that will select over SIGINT, SIGTERM and SIGTSTP signals.
    let signal_stream = Signal::new(unix_signal::SIGINT, &handle).flatten_stream()
        .select(Signal::new(unix_signal::SIGTERM, &handle).flatten_stream())
-        .select(Signal::new(20i32, &handle).flatten_stream()); // SIGTSTP
+        .select(Signal::new(libc::SIGTSTP, &handle).flatten_stream());

    // Execute the event loop that will listen for and transmit received
    // signals to the shell.
diff --git a/src/shell/flow.rs b/src/shell/flow.rs
@@ -10,6 +10,8 @@ use parser::{ForExpression, StatementSplitter, check_statement, expand_string, S
 use parser::peg::Pipeline;
 use super::assignments::{let_assignment, export_variable};
 use types::Array;
+#[cfg(not(target_os = "redox"))]
+use libc;

 pub enum Condition {
    Continue,
@@ -283,7 +285,7 @@ impl<'a> FlowLogic for Shell<'a> {
                _ => {}
            }
            if let Ok(signal) = self.signals.try_recv() {
-                if signal != 20 {
+                if cfg!(not(target_os = "redox")) && signal != libc::SIGTSTP {
                    self.handle_signal(signal);
                }
                return Condition::SigInt;
diff --git a/src/shell/job_control.rs b/src/shell/job_control.rs
@@ -1,4 +1,4 @@
-#[cfg(not(target_os = "redox"))] use libc::{pid_t, c_int};
+#[cfg(not(target_os = "redox"))] use libc::{self, pid_t, c_int};
 #[cfg(not(target_os = "redox"))] use nix::sys::signal::{self, Signal as NixSignal};
 use std::fmt;
 use std::io::{stderr, Write};
@@ -14,7 +14,7 @@ pub trait JobControl {
    fn handle_signal(&self, signal: i32);
    fn foreground_send(&self, signal: i32);
    fn background_send(&self, signal: i32);
-    fn send_child_to_background(&mut self, child: Child, state: ProcessState, offset: u32);
+    fn send_child_to_background(&mut self, child: Child, state: ProcessState);
 }

 #[derive(Clone)]
@@ -59,14 +59,15 @@ impl<'a> JobControl for Shell<'a> {
        // TODO: Redox doesn't support signals yet.
    }

+    #[cfg(not(target_os = "redox"))]
    /// Waits until all running background tasks have completed, and listens for signals in the
    /// event that a signal is sent to kill the running tasks.
    fn wait_for_background(&mut self) {
        'event: loop {
            for process in self.background.lock().unwrap().iter() {
                if let ProcessState::Running = process.state {
                    if let Ok(signal) = self.signals.try_recv() {
-                        if signal != 20 {
+                        if signal != libc::SIGTSTP {
                            self.background_send(signal);
                            process::exit(TERMINATED);
                        }
@@ -79,6 +80,11 @@ impl<'a> JobControl for Shell<'a> {
        }
    }

+    #[cfg(target_os = "redox")]
+    fn wait_for_background(&mut self) {
+        // TODO: Redox doesn't support signals yet.
+    }
+
    #[cfg(not(target_os = "redox"))]
    /// Send a kill signal to all running foreground tasks.
    fn foreground_send(&self, signal: i32) {
@@ -107,11 +113,8 @@ impl<'a> JobControl for Shell<'a> {
        // TODO: Redox doesn't support signals yet
    }

-    fn send_child_to_background(&mut self, mut child: Child, state: ProcessState, offset: u32) {
-        // NOTE: Why is this always off?
-        // command args + Ctrl + Z: off by 1
-        // commands args &: off by 2
-        let pid = child.id() + offset;
+    fn send_child_to_background(&mut self, mut child: Child, state: ProcessState) {
+        let pid = child.id();
        let processes = self.background.clone();
        let _ = spawn(move || {
            let njob;
@@ -162,8 +165,8 @@ impl<'a> JobControl for Shell<'a> {
    /// If a SIGTERM is received, a SIGTERM will be sent to all background processes
    /// before the shell terminates itself.
    fn handle_signal(&self, signal: i32) {
-        if signal == 15 {
-            self.background_send(15);
+        if signal == libc::SIGTERM {
+            self.background_send(libc::SIGTERM);
            process::exit(TERMINATED);
        }
    }
diff --git a/src/shell/pipe.rs b/src/shell/pipe.rs
@@ -1,3 +1,4 @@
+#[cfg(not(target_os = "redox"))] use libc;
 use std::io::{self, Write};
 use std::process::{Stdio, Command, Child};
 use std::os::unix::io::{FromRawFd, AsRawFd, IntoRawFd};
@@ -91,7 +92,7 @@ fn pipe(shell: &mut Shell, commands: &mut [(Command, JobKind)]) -> i32 {
        match kind {
            JobKind::Background => {
                if let Err(_) = command.spawn()
-                    .map(|child| shell.send_child_to_background(child, ProcessState::Running, 2))
+                    .map(|child| shell.send_child_to_background(child, ProcessState::Running))
                {
                    let stderr = io::stderr();
                    let mut stderr = stderr.lock();
@@ -174,7 +175,7 @@ fn pipe(shell: &mut Shell, commands: &mut [(Command, JobKind)]) -> i32 {
                }
                previous_status = wait(shell, &mut children);
                if previous_status == TERMINATED {
-                    shell.foreground_send(15);
+                    shell.foreground_send(libc::SIGTERM);
                    return previous_status;
                }
            }
@@ -214,10 +215,10 @@ fn wait_on_child(shell: &mut Shell, mut child: Child) -> i32 {
            },
            Ok(None) => {
                if let Ok(signal) = shell.signals.try_recv() {
-                    if signal == 20 {
+                    if signal == libc::SIGTSTP {
                        shell.received_sigtstp = true;
                        shell.suspend(child.id());
-                        shell.send_child_to_background(child, ProcessState::Stopped, 1);
+                        shell.send_child_to_background(child, ProcessState::Stopped);
                        break SUCCESS
                    } else {
                        if let Err(why) = child.kill() {
@@ -260,10 +261,10 @@ fn wait(shell: &mut Shell, children: &mut Vec<Option<Child>>) -> i32 {
                    },
                    Ok(None) => {
                        if let Ok(signal) = shell.signals.try_recv() {
-                            if signal == 20 {
+                            if signal == libc::SIGTSTP {
                                shell.received_sigtstp = true;
                                shell.suspend(child.id());
-                                shell.send_child_to_background(child, ProcessState::Stopped, 1);
+                                shell.send_child_to_background(child, ProcessState::Stopped);
                                break SUCCESS
                            }
                            shell.foreground_send(signal);
@@ -301,10 +302,10 @@ fn wait(shell: &mut Shell, children: &mut Vec<Option<Child>>) -> i32 {
                },
                Ok(None) => {
                    if let Ok(signal) = shell.signals.try_recv() {
-                        if signal == 20 {
+                        if signal == libc::SIGTSTP {
                            shell.received_sigtstp = true;
                            shell.suspend(child.id());
-                            shell.send_child_to_background(child, ProcessState::Stopped, 1);
+                            shell.send_child_to_background(child, ProcessState::Stopped);
                            break SUCCESS
                        }
                        shell.foreground_send(signal);