diff --git a/crates/runc/src/io.rs b/crates/runc/src/io.rs index dc501c9..63d6fd1 100644 --- a/crates/runc/src/io.rs +++ b/crates/runc/src/io.rs @@ -19,7 +19,7 @@ use std::io::Result; #[cfg(not(feature = "async"))] use std::io::{Read, Write}; use std::os::unix::fs::OpenOptionsExt; -use std::os::unix::io::{AsRawFd, FromRawFd}; +use std::os::unix::io::AsRawFd; use std::process::Stdio; use std::sync::Mutex; @@ -253,12 +253,8 @@ pub struct NullIo { impl NullIo { pub fn new() -> std::io::Result { - let fd = nix::fcntl::open( - "/dev/null", - nix::fcntl::OFlag::O_RDONLY, - nix::sys::stat::Mode::empty(), - )?; - let dev_null = unsafe { Mutex::new(Some(std::fs::File::from_raw_fd(fd))) }; + let f = OpenOptions::new().read(true).open("/dev/null")?; + let dev_null = Mutex::new(Some(f)); Ok(Self { dev_null }) } } diff --git a/crates/shim/src/asynchronous/mod.rs b/crates/shim/src/asynchronous/mod.rs index 1d5afd0..3ccb896 100644 --- a/crates/shim/src/asynchronous/mod.rs +++ b/crates/shim/src/asynchronous/mod.rs @@ -14,6 +14,7 @@ limitations under the License. */ +use std::convert::TryFrom; use std::os::unix::fs::FileTypeExt; use std::os::unix::io::AsRawFd; use std::os::unix::net::UnixListener; @@ -27,8 +28,12 @@ use std::{env, process}; use async_trait::async_trait; use command_fds::{CommandFdExt, FdMapping}; use futures::StreamExt; -use libc::{c_int, pid_t, SIGCHLD, SIGINT, SIGPIPE, SIGTERM}; +use libc::{SIGCHLD, SIGINT, SIGPIPE, SIGTERM}; use log::{debug, error, info, warn}; +use nix::errno::Errno; +use nix::sys::signal::Signal; +use nix::sys::wait::{self, WaitPidFlag, WaitStatus}; +use nix::unistd::Pid; use signal_hook_tokio::Signals; use tokio::io::AsyncWriteExt; use tokio::sync::Notify; @@ -304,28 +309,43 @@ async fn handle_signals(signals: Signals) { debug!("received {}", sig); } SIGCHLD => loop { - let result = asyncify(move || -> Result<(pid_t, c_int)> { - let mut status: c_int = -1; - let pid = unsafe { libc::waitpid(-1, &mut status, libc::WNOHANG) }; - if pid <= 0 { - return Err(other!("wait finished")); - } - let status = libc::WEXITSTATUS(status); - Ok((pid, status)) + // Note: see comment at the counterpart in synchronous/mod.rs for details. + match asyncify(move || { + Ok(wait::waitpid( + Some(Pid::from_raw(-1)), + Some(WaitPidFlag::WNOHANG), + )?) }) - .await; - - if let Ok((res_pid, status)) = result { - monitor_notify_by_pid(res_pid, status) - .await - .unwrap_or_else(|e| { - error!("failed to send pid exit event {}", e); - }) - } else { - break; + .await + { + Ok(WaitStatus::Exited(pid, status)) => { + monitor_notify_by_pid(pid.as_raw(), status) + .await + .unwrap_or_else(|e| error!("failed to send exit event {}", e)) + } + Ok(WaitStatus::Signaled(pid, sig, _)) => { + debug!("child {} terminated({})", pid, sig); + let exit_code = 128 + sig as i32; + monitor_notify_by_pid(pid.as_raw(), exit_code) + .await + .unwrap_or_else(|e| error!("failed to send signal event {}", e)) + } + Err(Error::Nix(Errno::ECHILD)) => { + break; + } + Err(e) => { + warn!("error occurred in signal handler: {}", e); + } + _ => {} } }, - _ => {} + _ => { + if let Ok(sig) = Signal::try_from(sig) { + debug!("received {}", sig); + } else { + warn!("received invalid signal {}", sig); + } + } } } } diff --git a/crates/shim/src/synchronous/mod.rs b/crates/shim/src/synchronous/mod.rs index f29e0b4..4717d51 100644 --- a/crates/shim/src/synchronous/mod.rs +++ b/crates/shim/src/synchronous/mod.rs @@ -32,6 +32,7 @@ //! ``` //! +use std::convert::TryFrom; use std::env; use std::fs; use std::io::Write; @@ -42,8 +43,12 @@ use std::process::{self, Command, Stdio}; use std::sync::{Arc, Condvar, Mutex}; use command_fds::{CommandFdExt, FdMapping}; -use libc::{c_int, pid_t, SIGCHLD, SIGINT, SIGPIPE, SIGTERM}; +use libc::{SIGCHLD, SIGINT, SIGPIPE, SIGTERM}; pub use log::{debug, error, info, warn}; +use nix::errno::Errno; +use nix::sys::signal::Signal; +use nix::sys::wait::{self, WaitPidFlag, WaitStatus}; +use nix::unistd::Pid; use signal_hook::iterator::Signals; use crate::protos::protobuf::Message; @@ -230,23 +235,34 @@ fn handle_signals(mut signals: Signals) { debug!("received {}", sig); } SIGCHLD => loop { - unsafe { - let pid: pid_t = -1; - let mut status: c_int = 0; - let options: c_int = libc::WNOHANG; - let res_pid = libc::waitpid(pid, &mut status, options); - let status = libc::WEXITSTATUS(status); - if res_pid <= 0 { - break; - } else { - monitor::monitor_notify_by_pid(res_pid, status).unwrap_or_else(|e| { - error!("failed to send exit event {}", e); - }); + // Note that this thread sticks to child even it is suspended. + match wait::waitpid(Some(Pid::from_raw(-1)), Some(WaitPidFlag::WNOHANG)) { + Ok(WaitStatus::Exited(pid, status)) => { + monitor::monitor_notify_by_pid(pid.as_raw(), status) + .unwrap_or_else(|e| error!("failed to send exit event {}", e)) } + Ok(WaitStatus::Signaled(pid, sig, _)) => { + debug!("child {} terminated({})", pid, sig); + let exit_code = 128 + sig as i32; + monitor::monitor_notify_by_pid(pid.as_raw(), exit_code) + .unwrap_or_else(|e| error!("failed to send signal event {}", e)) + } + Err(Errno::ECHILD) => { + break; + } + Err(e) => { + // stick until all children will be successfully waited, even some unexpected error occurs. + warn!("error occurred in signal handler: {}", e); + } + _ => {} // stick until exit } }, _ => { - debug!("received {}", sig); + if let Ok(sig) = Signal::try_from(sig) { + debug!("received {}", sig); + } else { + warn!("received invalid signal {}", sig); + } } } }