Skip to content
Permalink
Browse files

Fix EAGAIN Fork Errors

The `waitpid()` function is required to reap child processes. As the
forking code was not using it, it would eventually cause EAGAIN errors
to occur. With this change, this will no longer be the case.

Closes #589
  • Loading branch information...
mmstick committed Dec 27, 2017
1 parent 1665857 commit bc49acb6b9555942ba8d568914d61389052e9b2b
Showing with 74 additions and 26 deletions.
  1. +1 −1 src/shell/binary/prompt.rs
  2. +32 −19 src/shell/fork.rs
  3. +1 −1 src/shell/mod.rs
  4. +1 −5 src/shell/pipe_exec/fork.rs
  5. +19 −0 src/sys/redox.rs
  6. +20 −0 src/sys/unix/mod.rs
@@ -24,7 +24,7 @@ pub(crate) fn prompt_fn(shell: &mut Shell) -> Option<String> {
let _ = function.read().execute(child, &["ion"]);
}) {
Ok(result) => {
let mut string = String::new();
let mut string = String::with_capacity(1024);
match result.stdout.unwrap().read_to_string(&mut string) {
Ok(_) => output = Some(string),
Err(why) => {
@@ -1,7 +1,6 @@
use super::{IonError, Shell};
use std::fs::File;
use std::os::unix::io::{AsRawFd, FromRawFd};
use std::process::exit;
use sys;

#[repr(u8)]
@@ -47,6 +46,7 @@ pub struct IonResult {
pub pid: u32,
pub stdout: Option<File>,
pub stderr: Option<File>,
pub status: u8,
}

impl<'a> Fork<'a> {
@@ -62,7 +62,7 @@ impl<'a> Fork<'a> {
let mut outs = if self.capture as u8 & Capture::Stdout as u8 != 0 {
Some(sys::pipe2(sys::O_CLOEXEC)
.map(|fds| unsafe { (File::from_raw_fd(fds.0), File::from_raw_fd(fds.1)) })
.map_err(|err| IonError::Fork { why: err })?)
.map_err(|why| IonError::Fork { why })?)
} else {
None
};
@@ -71,7 +71,7 @@ impl<'a> Fork<'a> {
let mut errs = if self.capture as u8 & Capture::Stderr as u8 != 0 {
Some(sys::pipe2(sys::O_CLOEXEC)
.map(|fds| unsafe { (File::from_raw_fd(fds.0), File::from_raw_fd(fds.1)) })
.map_err(|err| IonError::Fork { why: err })?)
.map_err(|why| IonError::Fork { why })?)
} else {
None
};
@@ -104,25 +104,38 @@ impl<'a> Fork<'a> {
let _ = sys::dup2(write.as_raw_fd(), sys::STDERR_FILENO);
}

// Execute the given closure within the child's shell.
// Drop all the file descriptors that we no longer need.
drop(null_file);
drop(outs);
drop(errs);

// Obtain ownership of the child's copy of the shell, and then configure it.
let mut shell: Shell = unsafe { (self.shell as *const Shell).read() };
child_func(&mut shell);
shell.set_var("PID", &sys::getpid().unwrap_or(0).to_string());
let _ = shell.context.take();

// Reap the child, enabling the parent to get EOF from the read end of the pipe.
exit(shell.previous_status);
// Execute the given closure within the child's shell.
child_func(&mut shell);
sys::fork_exit(shell.previous_status);
}
Ok(pid) => Ok(IonResult {
pid,
stdout: outs.map(|(read, write)| {
drop(write);
read
}),
stderr: errs.map(|(read, write)| {
drop(write);
read
}),
}),
Err(why) => Err(IonError::Fork { why: why }),
Ok(pid) => {
Ok(IonResult {
pid,
stdout: outs.map(|(read, write)| {
drop(write);
read
}),
stderr: errs.map(|(read, write)| {
drop(write);
read
}),
// `waitpid()` is required to reap the child.
status: sys::wait_for_child(pid).map_err(|why| IonError::Fork { why })?,
})
},
Err(why) => {
Err(IonError::Fork { why: why })
},
}
}
}
@@ -463,7 +463,7 @@ impl<'a> Expander for Shell {
let mut output = None;
match self.fork(Capture::StdoutThenIgnoreStderr, move |shell| shell.on_command(command)) {
Ok(result) => {
let mut string = String::new();
let mut string = String::with_capacity(1024);
match result.stdout.unwrap().read_to_string(&mut string) {
Ok(_) => output = Some(string),
Err(why) => {
@@ -29,12 +29,8 @@ pub(crate) fn fork_pipe(
// This ensures that the child fork has a unique PGID.
create_process_group(0);

// Drop the context and it's background thread in the child.
// NOTE: Solves a memory leak.
// shell.context.take().map(|mut c| c.history.commit_history());

// After execution of it's commands, exit with the last command's status.
exit(pipe(shell, commands, false));
sys::fork_exit(pipe(shell, commands, false));
}
Ok(pid) => {
if state != ProcessState::Empty {
@@ -31,6 +31,25 @@ pub(crate) fn is_root() -> bool { syscall::geteuid().map(|id| id == 0).unwrap_or

pub unsafe fn fork() -> io::Result<u32> { cvt(syscall::clone(0)).map(|pid| pid as u32) }

pub fn fork_exit(status: usize) -> ! { syscall::exit(status) }

pub fn wait_for_child(pid: u32) -> io::Result<u8> {
let mut status;
use syscall::{waitpid, ECHILD};

loop {
status = 0;
match unsafe { waitpid(pid as usize, &mut status, 0) } {
Err(error) if error.errno == ECHILD => break,
Err(error) => return Err(io::Error::from_raw_os_error(error.errono)),
=> ()
}
}

// Ok(WEXITSTATUS(status) as u8)
Ok(0)
}

pub(crate) fn getpid() -> io::Result<u32> { cvt(syscall::getpid()).map(|pid| pid as u32) }

pub(crate) fn kill(pid: u32, signal: i32) -> io::Result<()> {
@@ -24,6 +24,8 @@ pub(crate) const STDOUT_FILENO: i32 = libc::STDOUT_FILENO;
pub(crate) const STDERR_FILENO: i32 = libc::STDERR_FILENO;
pub(crate) const STDIN_FILENO: i32 = libc::STDIN_FILENO;

fn errno() -> i32 { unsafe { *libc::__errno_location() } }

pub(crate) fn geteuid() -> io::Result<u32> { Ok(unsafe { libc::geteuid() } as u32) }

pub(crate) fn getuid() -> io::Result<u32> { Ok(unsafe { libc::getuid() } as u32) }
@@ -32,6 +34,24 @@ pub(crate) fn is_root() -> bool { unsafe { libc::geteuid() == 0 } }

pub unsafe fn fork() -> io::Result<u32> { cvt(libc::fork()).map(|pid| pid as u32) }

pub fn wait_for_child(pid: u32) -> io::Result<u8> {
let mut status;
use libc::{waitpid, ECHILD, WEXITSTATUS};

loop {
status = 0;
match unsafe { waitpid(pid as i32, &mut status, 0) } {
-1 if errno() == ECHILD => break,
-1 => return Err(io::Error::from_raw_os_error(errno())),
_ => ()
}
}

Ok(unsafe { WEXITSTATUS(status) as u8 })
}

pub fn fork_exit(exit_status: i32) -> ! { unsafe { libc::_exit(exit_status) } }

pub(crate) fn getpid() -> io::Result<u32> { cvt(unsafe { libc::getpid() }).map(|pid| pid as u32) }

pub(crate) fn kill(pid: u32, signal: i32) -> io::Result<()> {

0 comments on commit bc49acb

Please sign in to comment.
You can’t perform that action at this time.