fix restoring stdio in tests

Signed-off-by: Jorge Prendes <[email protected]>

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<Stdio> = OnceLock::new();
+
 impl Stdio {
     pub fn redirect(self) -> Result<()> {
         self.stdin.redirect()?;
@@ -39,45 +38,74 @@ 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<const FD: StdioRawFd>(Arc<AtomicCell<Option<File>>>);
+pub struct StdioStream<const FD: StdioRawFd>(Arc<StdioOwnedFd>);
 
 impl<const FD: StdioRawFd> StdioStream<FD> {
     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());
             }
         }
         Ok(())
     }
 
     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<Self> {
+        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<P: AsRef<Path>, const FD: StdioRawFd> TryFrom<Option<P>> for StdioStream<FD> {
     type Error = Error;
     fn try_from(path: Option<P>) -> Result<Self> {
-        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)))
     }
 }
 

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<OwnedFd>) -> Result<impl StdioAsRawFd> {
-    Ok(f.into())
+pub type StdioRawFd = RawFd;
+
+pub struct StdioOwnedFd(AtomicCell<StdioRawFd>);
+
+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<P: AsRef<Path>>(path: P) -> Result<File> {
-    OpenOptions::new().read(true).write(true).open(path)
+impl StdioOwnedFd {
+    pub fn try_from(f: impl Into<OwnedFd>) -> Result<Self> {
+        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<StdioRawFd> {
+        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<Path>) -> Result<Self> {
+        Self::try_from(OpenOptions::new().read(true).write(true).open(path)?)
+    }
 }

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<StdioRawFd>);
 
-pub fn try_into_fd(f: impl Into<OwnedHandle>) -> Result<impl StdioAsRawFd> {
-    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<P: AsRef<Path>>(path: P) -> Result<File> {
-    // 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<OwnedHandle>) -> Result<Self> {
+        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<StdioRawFd> {
+        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<Path>) -> Result<Self> {
+        // 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)?)
     }
 }

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"]