diff --git a/Cargo.lock b/Cargo.lock index 2fd1afa80..a0f59f22e 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -622,10 +622,10 @@ dependencies = [ "containerd-shim", "containerd-shim-wasm", "dbus", + "env_logger", "libc", "libcontainer", "log", - "nix 0.26.4", "oci-spec", "serde_json", "serial_test", @@ -672,10 +672,8 @@ dependencies = [ "containerd-shim-wasm", "dbus", "env_logger", - "libc", "libcontainer", "log", - "nix 0.26.4", "oci-spec", "serde", "serde_json", diff --git a/crates/containerd-shim-wasm/src/sandbox/stdio.rs b/crates/containerd-shim-wasm/src/sandbox/stdio.rs index 9354f3375..7f95a2ab4 100644 --- a/crates/containerd-shim-wasm/src/sandbox/stdio.rs +++ b/crates/containerd-shim-wasm/src/sandbox/stdio.rs @@ -1,10 +1,7 @@ -use std::fs::File; use std::io::ErrorKind::NotFound; use std::io::{Error, Result}; use std::path::Path; -use std::sync::Arc; - -use crossbeam::atomic::AtomicCell; +use std::sync::{Arc, OnceLock}; use super::InstanceConfig; use crate::sys::stdio::*; @@ -16,6 +13,8 @@ pub struct Stdio { pub stderr: Stderr, } +static INITIAL_STDIO: OnceLock = OnceLock::new(); + impl Stdio { pub fn redirect(self) -> Result<()> { self.stdin.redirect()?; @@ -39,17 +38,39 @@ impl Stdio { stderr: cfg.get_stderr().try_into()?, }) } + + pub fn init_from_std() -> Self { + Self { + stdin: Stdin::try_from_std().unwrap_or_default(), + stdout: Stdout::try_from_std().unwrap_or_default(), + stderr: Stderr::try_from_std().unwrap_or_default(), + } + } + + pub fn guard(self) -> impl Drop { + StdioGuard(self) + } +} + +struct StdioGuard(Stdio); + +impl Drop for StdioGuard { + fn drop(&mut self) { + let _ = self.0.take().redirect(); + } } #[derive(Clone, Default)] -pub struct StdioStream(Arc>>); +pub struct StdioStream(Arc); impl StdioStream { pub fn redirect(self) -> Result<()> { - if let Some(f) = self.0.take() { - let f = try_into_fd(f)?; - let _ = unsafe { libc::dup(FD) }; - if unsafe { libc::dup2(f.as_raw_fd(), FD) } == -1 { + if let Some(fd) = self.0.as_raw_fd() { + // Before any redirection we try to keep a copy of the original stdio + // to make sure the streams stay open + INITIAL_STDIO.get_or_init(Stdio::init_from_std); + + if unsafe { libc::dup2(fd, FD) } == -1 { return Err(Error::last_os_error()); } } @@ -57,27 +78,34 @@ impl StdioStream { } pub fn take(&self) -> Self { - Self(Arc::new(AtomicCell::new(self.0.take()))) + Self(Arc::new(self.0.take())) + } + + pub fn try_from_std() -> Result { + let fd: i32 = unsafe { libc::dup(FD) }; + if fd == -1 { + return Err(Error::last_os_error()); + } + Ok(Self(Arc::new(unsafe { StdioOwnedFd::from_raw_fd(fd) }))) } } impl, const FD: StdioRawFd> TryFrom> for StdioStream { type Error = Error; fn try_from(path: Option

) -> Result { - let file = path + let fd = path .and_then(|path| match path.as_ref() { path if path.as_os_str().is_empty() => None, - path => Some(path.to_owned()), - }) - .map(|path| match open_file(path) { - Err(err) if err.kind() == NotFound => Ok(None), - Ok(f) => Ok(Some(f)), - Err(err) => Err(err), + path => Some(StdioOwnedFd::try_from_path(path)), }) - .transpose()? - .flatten(); + .transpose() + .or_else(|err| match err.kind() { + NotFound => Ok(None), + _ => Err(err), + })? + .unwrap_or_default(); - Ok(Self(Arc::new(AtomicCell::new(file)))) + Ok(Self(Arc::new(fd))) } } diff --git a/crates/containerd-shim-wasm/src/sys/unix/stdio.rs b/crates/containerd-shim-wasm/src/sys/unix/stdio.rs index 3bd0ae0f9..51f4e9d8d 100644 --- a/crates/containerd-shim-wasm/src/sys/unix/stdio.rs +++ b/crates/containerd-shim-wasm/src/sys/unix/stdio.rs @@ -1,15 +1,51 @@ -use std::fs::{File, OpenOptions}; +use std::fs::OpenOptions; use std::io::Result; -use std::os::fd::OwnedFd; -pub use std::os::fd::{AsRawFd as StdioAsRawFd, RawFd as StdioRawFd}; +use std::os::fd::{IntoRawFd, OwnedFd, RawFd}; use std::path::Path; +use crossbeam::atomic::AtomicCell; pub use libc::{STDERR_FILENO, STDIN_FILENO, STDOUT_FILENO}; -pub fn try_into_fd(f: impl Into) -> Result { - Ok(f.into()) +pub type StdioRawFd = RawFd; + +pub struct StdioOwnedFd(AtomicCell); + +impl Drop for StdioOwnedFd { + fn drop(&mut self) { + let fd = self.0.swap(-1); + if fd >= 0 { + unsafe { libc::close(fd) }; + } + } +} + +impl Default for StdioOwnedFd { + fn default() -> Self { + Self(AtomicCell::new(-1)) + } } -pub fn open_file>(path: P) -> Result { - OpenOptions::new().read(true).write(true).open(path) +impl StdioOwnedFd { + pub fn try_from(f: impl Into) -> Result { + let fd = f.into().into_raw_fd(); + Ok(unsafe { Self::from_raw_fd(fd) }) + } + + pub unsafe fn from_raw_fd(fd: StdioRawFd) -> Self { + Self(AtomicCell::new(fd)) + } + + pub fn as_raw_fd(&self) -> Option { + let fd = self.0.load(); + (fd >= 0).then_some(fd) + } + + pub fn take(&self) -> Self { + let fd = self.0.swap(-1); + unsafe { Self::from_raw_fd(fd) } + } + + pub fn try_from_path(path: impl AsRef) -> Result { + Self::try_from(OpenOptions::new().read(true).write(true).open(path)?) + } } diff --git a/crates/containerd-shim-wasm/src/sys/windows/stdio.rs b/crates/containerd-shim-wasm/src/sys/windows/stdio.rs index 1c92d1c71..fb763b1b2 100644 --- a/crates/containerd-shim-wasm/src/sys/windows/stdio.rs +++ b/crates/containerd-shim-wasm/src/sys/windows/stdio.rs @@ -1,11 +1,12 @@ -use std::fs::{File, OpenOptions}; +use std::fs::OpenOptions; use std::io::ErrorKind::Other; use std::io::{Error, Result}; use std::os::windows::fs::OpenOptionsExt; use std::os::windows::prelude::{AsRawHandle, IntoRawHandle, OwnedHandle}; use std::path::Path; -use libc::{c_int, close, intptr_t, open_osfhandle, O_APPEND}; +use crossbeam::atomic::AtomicCell; +use libc::{intptr_t, open_osfhandle, O_APPEND}; use windows_sys::Win32::Storage::FileSystem::FILE_FLAG_OVERLAPPED; pub type StdioRawFd = libc::c_int; @@ -14,41 +15,55 @@ pub const STDIN_FILENO: StdioRawFd = 0; pub const STDOUT_FILENO: StdioRawFd = 1; pub const STDERR_FILENO: StdioRawFd = 2; -struct StdioOwnedFd(c_int); +pub struct StdioOwnedFd(AtomicCell); -pub fn try_into_fd(f: impl Into) -> Result { - let handle = f.into(); - let fd = unsafe { open_osfhandle(handle.as_raw_handle() as intptr_t, O_APPEND) }; - if fd == -1 { - return Err(Error::new(Other, "Failed to open file descriptor")); +impl Drop for StdioOwnedFd { + fn drop(&mut self) { + let fd = self.0.swap(-1); + if fd >= 0 { + unsafe { libc::close(fd) }; + } } - let _ = handle.into_raw_handle(); // drop ownership of the handle, it's managed by fd now - Ok(StdioOwnedFd(fd)) } -pub fn open_file>(path: P) -> Result { - // Containerd always passes a named pipe for stdin, stdout, and stderr so we can check if it is a pipe and open with overlapped IO - let mut options = OpenOptions::new(); - options.read(true).write(true); - if path.as_ref().starts_with("\\\\.\\pipe\\") { - options.custom_flags(FILE_FLAG_OVERLAPPED); +impl Default for StdioOwnedFd { + fn default() -> Self { + Self(AtomicCell::new(-1)) } - - options.open(path) } -pub trait StdioAsRawFd { - fn as_raw_fd(&self) -> c_int; -} +impl StdioOwnedFd { + pub fn try_from(f: impl Into) -> Result { + let handle = f.into(); + let fd = unsafe { open_osfhandle(handle.as_raw_handle() as intptr_t, O_APPEND) }; + if fd == -1 { + return Err(Error::new(Other, "Failed to open file descriptor")); + } + let _ = handle.into_raw_handle(); // drop ownership of the handle, it's managed by fd now + Ok(unsafe { Self::from_raw_fd(fd) }) + } -impl StdioAsRawFd for StdioOwnedFd { - fn as_raw_fd(&self) -> c_int { - self.0 + pub unsafe fn from_raw_fd(fd: StdioRawFd) -> Self { + Self(AtomicCell::new(fd)) } -} -impl Drop for StdioOwnedFd { - fn drop(&mut self) { - unsafe { close(self.0) }; + pub fn as_raw_fd(&self) -> Option { + let fd = self.0.load(); + (fd >= 0).then_some(fd) + } + + pub fn take(&self) -> Self { + let fd = self.0.swap(-1); + unsafe { Self::from_raw_fd(fd) } + } + + pub fn try_from_path(path: impl AsRef) -> Result { + // Containerd always passes a named pipe for stdin, stdout, and stderr so we can check if it is a pipe and open with overlapped IO + let mut options = OpenOptions::new(); + options.read(true).write(true); + if path.as_ref().starts_with(r"\\.\pipe\") { + options.custom_flags(FILE_FLAG_OVERLAPPED); + } + Self::try_from(options.open(path)?) } } diff --git a/crates/containerd-shim-wasmedge/Cargo.toml b/crates/containerd-shim-wasmedge/Cargo.toml index 740132d84..06c635a17 100644 --- a/crates/containerd-shim-wasmedge/Cargo.toml +++ b/crates/containerd-shim-wasmedge/Cargo.toml @@ -15,16 +15,16 @@ anyhow = { workspace = true } oci-spec = { workspace = true, features = ["runtime"] } thiserror = { workspace = true } serde_json = { workspace = true } -libc = { workspace = true } [target.'cfg(unix)'.dependencies] libcontainer = { workspace = true } dbus = { version = "*", optional = true } -nix = { workspace = true } [dev-dependencies] -tempfile = "3.7" +tempfile = "3.8" serial_test = "*" +env_logger = "0.10" +libc = { workspace = true } [features] default = ["standalone", "static"] diff --git a/crates/containerd-shim-wasmedge/src/instance/instance_linux.rs b/crates/containerd-shim-wasmedge/src/instance/instance_linux.rs index 9e4608eac..dc89086fa 100644 --- a/crates/containerd-shim-wasmedge/src/instance/instance_linux.rs +++ b/crates/containerd-shim-wasmedge/src/instance/instance_linux.rs @@ -77,38 +77,18 @@ impl LibcontainerInstance for Wasi { mod wasitest { use std::fs::read_to_string; - use std::os::unix::io::RawFd; use containerd_shim_wasm::function; use containerd_shim_wasm::sandbox::testutil::{ has_cap_sys_admin, run_test_with_sudo, run_wasi_test, }; use containerd_shim_wasm::sandbox::Instance; - use libc::{dup2, STDERR_FILENO, STDIN_FILENO, STDOUT_FILENO}; use serial_test::serial; use tempfile::tempdir; use wasmedge_sdk::wat2wasm; use super::*; - static mut STDIN_FD: Option = None; - static mut STDOUT_FD: Option = None; - static mut STDERR_FD: Option = None; - - fn reset_stdio() { - unsafe { - if let Some(stdin) = STDIN_FD { - let _ = dup2(stdin, STDIN_FILENO); - } - if let Some(stdout) = STDOUT_FD { - let _ = dup2(stdout, STDOUT_FILENO); - } - if let Some(stderr) = STDERR_FD { - let _ = dup2(stderr, STDERR_FILENO); - } - } - } - // This is taken from https://github.com/bytecodealliance/wasmtime/blob/6a60e8363f50b936e4c4fc958cb9742314ff09f3/docs/WASI-tutorial.md?plain=1#L270-L298 const WASI_HELLO_WAT: &[u8]= r#"(module ;; Import the required fd_write WASI function which will write the given io vectors to stdout @@ -185,6 +165,13 @@ mod wasitest { return run_test_with_sudo(function!()); } + // start logging + // to enable logging run `export RUST_LOG=trace` and append cargo command with + // --show-output before running test + let _ = env_logger::try_init(); + + let _guard = Stdio::init_from_std().guard(); + let dir = tempdir()?; let path = dir.path(); let wasm_bytes = wat2wasm(WASI_HELLO_WAT).unwrap(); @@ -196,7 +183,6 @@ mod wasitest { let output = read_to_string(path.join("stdout"))?; assert_eq!(output, "hello world\n"); - reset_stdio(); Ok(()) } @@ -208,6 +194,11 @@ mod wasitest { return run_test_with_sudo(function!()); } + // start logging + let _ = env_logger::try_init(); + + let _guard = Stdio::init_from_std().guard(); + let dir = tempdir()?; let wasm_bytes = wat2wasm(WASI_RETURN_ERROR).unwrap(); @@ -216,7 +207,6 @@ mod wasitest { // Expect error code from the run. assert_eq!(res.0, 137); - reset_stdio(); Ok(()) } @@ -228,15 +218,21 @@ mod wasitest { return run_test_with_sudo(function!()); } + // start logging + let _ = env_logger::try_init(); + + let _guard = Stdio::init_from_std().guard(); + let expected_exit_code: u32 = 42; let dir = tempdir()?; let wasm_bytes = module_with_exit_code(expected_exit_code); + let wasm_bytes = wat2wasm(&wasm_bytes).unwrap(); + let (actual_exit_code, _) = run_wasi_test::(&dir, wasm_bytes, None)?; assert_eq!(actual_exit_code, expected_exit_code); - reset_stdio(); Ok(()) } } diff --git a/crates/containerd-shim-wasmer/Cargo.toml b/crates/containerd-shim-wasmer/Cargo.toml index 550fc5afe..159c9c8c5 100644 --- a/crates/containerd-shim-wasmer/Cargo.toml +++ b/crates/containerd-shim-wasmer/Cargo.toml @@ -25,7 +25,6 @@ nix = { workspace = true } [dev-dependencies] tempfile = "3.7" -libc = { workspace = true } env_logger = "0.10" serial_test = "*" diff --git a/crates/containerd-shim-wasmer/src/instance/instance_linux.rs b/crates/containerd-shim-wasmer/src/instance/instance_linux.rs index 55ebefa10..4ef1a27ec 100644 --- a/crates/containerd-shim-wasmer/src/instance/instance_linux.rs +++ b/crates/containerd-shim-wasmer/src/instance/instance_linux.rs @@ -87,38 +87,17 @@ struct Options { mod wasitest { use std::fs::read_to_string; - use std::os::fd::RawFd; use containerd_shim_wasm::function; use containerd_shim_wasm::sandbox::testutil::{ has_cap_sys_admin, run_test_with_sudo, run_wasi_test, }; use containerd_shim_wasm::sandbox::Instance; - use libc::{STDERR_FILENO, STDIN_FILENO, STDOUT_FILENO}; - use nix::unistd::dup2; use serial_test::serial; use tempfile::tempdir; use super::*; - static mut STDIN_FD: Option = None; - static mut STDOUT_FD: Option = None; - static mut STDERR_FD: Option = None; - - fn reset_stdio() { - unsafe { - if let Some(stdin) = STDIN_FD { - let _ = dup2(stdin, STDIN_FILENO); - } - if let Some(stdout) = STDOUT_FD { - let _ = dup2(stdout, STDOUT_FILENO); - } - if let Some(stderr) = STDERR_FD { - let _ = dup2(stderr, STDERR_FILENO); - } - } - } - // This is taken from https://github.com/bytecodealliance/wasmtime/blob/6a60e8363f50b936e4c4fc958cb9742314ff09f3/docs/WASI-tutorial.md?plain=1#L270-L298 fn hello_world_module(start_fn: Option<&str>) -> Vec { let start_fn = start_fn.unwrap_or("_start"); @@ -179,7 +158,6 @@ mod wasitest { let i = Wasi::new("".to_string(), Some(&cfg)); i.delete()?; - reset_stdio(); Ok(()) } @@ -190,11 +168,14 @@ mod wasitest { println!("running test with sudo: {}", function!()); return run_test_with_sudo(function!()); } + // start logging // to enable logging run `export RUST_LOG=trace` and append cargo command with // --show-output before running test let _ = env_logger::try_init(); + let _guard = Stdio::init_from_std().guard(); + let dir = tempdir()?; let path = dir.path(); let wasm_bytes = hello_world_module(None); @@ -206,7 +187,6 @@ mod wasitest { let output = read_to_string(path.join("stdout"))?; assert_eq!(output, "hello world\n"); - reset_stdio(); Ok(()) } @@ -218,9 +198,12 @@ mod wasitest { println!("running test with sudo: {}", function!()); return run_test_with_sudo(function!()); } + // start logging let _ = env_logger::try_init(); + let _guard = Stdio::init_from_std().guard(); + let dir = tempdir()?; let path = dir.path(); let wasm_bytes = hello_world_module(Some("foo")); @@ -232,7 +215,6 @@ mod wasitest { let output = read_to_string(path.join("stdout"))?; assert_eq!(output, "hello world\n"); - reset_stdio(); Ok(()) } @@ -247,16 +229,16 @@ mod wasitest { // start logging let _ = env_logger::try_init(); + let _guard = Stdio::init_from_std().guard(); + let expected_exit_code: u32 = 42; let dir = tempdir()?; let wasm_bytes = module_with_exit_code(expected_exit_code); - log::info!("{:?}", wasm_bytes); let (actual_exit_code, _) = run_wasi_test::(&dir, wasm_bytes, None)?; assert_eq!(actual_exit_code, expected_exit_code); - reset_stdio(); Ok(()) } } diff --git a/crates/containerd-shim-wasmtime/Cargo.toml b/crates/containerd-shim-wasmtime/Cargo.toml index 199d0cb0c..d1b2bf815 100644 --- a/crates/containerd-shim-wasmtime/Cargo.toml +++ b/crates/containerd-shim-wasmtime/Cargo.toml @@ -33,16 +33,13 @@ oci-spec = { workspace = true, features = ["runtime"] } thiserror = { workspace = true } serde_json = { workspace = true } serde = { workspace = true } -libc = { workspace = true } [target.'cfg(unix)'.dependencies] -nix = { workspace = true } libcontainer = { workspace = true } dbus = { version = "*", optional = true } [dev-dependencies] tempfile = "3.8" -libc = { workspace = true } serial_test = "*" env_logger = "0.10" diff --git a/crates/containerd-shim-wasmtime/src/instance/instance_linux.rs b/crates/containerd-shim-wasmtime/src/instance/instance_linux.rs index 4437b31a4..542b48ae2 100644 --- a/crates/containerd-shim-wasmtime/src/instance/instance_linux.rs +++ b/crates/containerd-shim-wasmtime/src/instance/instance_linux.rs @@ -82,38 +82,17 @@ impl LibcontainerInstance for Wasi { mod wasitest { use std::fs::read_to_string; - use std::os::fd::RawFd; use containerd_shim_wasm::function; use containerd_shim_wasm::sandbox::testutil::{ has_cap_sys_admin, run_test_with_sudo, run_wasi_test, }; use containerd_shim_wasm::sandbox::Instance; - use libc::{STDERR_FILENO, STDIN_FILENO, STDOUT_FILENO}; - use nix::unistd::dup2; use serial_test::serial; use tempfile::tempdir; use super::*; - static mut STDIN_FD: Option = None; - static mut STDOUT_FD: Option = None; - static mut STDERR_FD: Option = None; - - fn reset_stdio() { - unsafe { - if let Some(stdin) = STDIN_FD { - let _ = dup2(stdin, STDIN_FILENO); - } - if let Some(stdout) = STDOUT_FD { - let _ = dup2(stdout, STDOUT_FILENO); - } - if let Some(stderr) = STDERR_FD { - let _ = dup2(stderr, STDERR_FILENO); - } - } - } - // This is taken from https://github.com/bytecodealliance/wasmtime/blob/6a60e8363f50b936e4c4fc958cb9742314ff09f3/docs/WASI-tutorial.md?plain=1#L270-L298 fn hello_world_module(start_fn: Option<&str>) -> Vec { let start_fn = start_fn.unwrap_or("_start"); @@ -174,7 +153,6 @@ mod wasitest { let i = Wasi::new("".to_string(), Some(&cfg)); i.delete()?; - reset_stdio(); Ok(()) } @@ -185,11 +163,14 @@ mod wasitest { println!("running test with sudo: {}", function!()); return run_test_with_sudo(function!()); } + // start logging // to enable logging run `export RUST_LOG=trace` and append cargo command with // --show-output before running test let _ = env_logger::try_init(); + let _guard = Stdio::init_from_std().guard(); + let dir = tempdir()?; let path = dir.path(); let wasm_bytes = hello_world_module(None); @@ -201,7 +182,6 @@ mod wasitest { let output = read_to_string(path.join("stdout"))?; assert_eq!(output, "hello world\n"); - reset_stdio(); Ok(()) } @@ -217,6 +197,8 @@ mod wasitest { // start logging let _ = env_logger::try_init(); + let _guard = Stdio::init_from_std().guard(); + let dir = tempdir()?; let path = dir.path(); let wasm_bytes = hello_world_module(Some("foo")); @@ -228,7 +210,6 @@ mod wasitest { let output = read_to_string(path.join("stdout"))?; assert_eq!(output, "hello world\n"); - reset_stdio(); Ok(()) } @@ -243,6 +224,8 @@ mod wasitest { // start logging let _ = env_logger::try_init(); + let _guard = Stdio::init_from_std().guard(); + let expected_exit_code: u32 = 42; let dir = tempdir()?; @@ -251,7 +234,6 @@ mod wasitest { assert_eq!(actual_exit_code, expected_exit_code); - reset_stdio(); Ok(()) } }