Use an RAII wrapper for stdio redirection

crates/containerd-shim-wasm/src/lib.rs

@@ -5,9 +5,9 @@
 
 pub mod sandbox;
 pub mod services;
-#[cfg_attr(unix, path = "sys/unix.rs")]
-#[cfg_attr(windows, path = "sys/windows.rs")]
-pub mod sys;
+#[cfg_attr(unix, path = "sys/unix/mod.rs")]
+#[cfg_attr(windows, path = "sys/windows/mod.rs")]
+pub(crate) mod sys;
 
 #[cfg(all(feature = "libcontainer", not(target_os = "windows")))]
 pub mod libcontainer_instance;

crates/containerd-shim-wasm/src/libcontainer_instance/container_executor.rs

@@ -1,38 +1,31 @@
 use std::fs::OpenOptions;
 use std::io::Read;
-use std::os::fd::RawFd;
 use std::path::PathBuf;
 
-use libc::{STDERR_FILENO, STDIN_FILENO, STDOUT_FILENO};
 use libcontainer::workload::default::DefaultExecutor;
 use libcontainer::workload::{Executor, ExecutorError};
-use nix::unistd::{dup, dup2};
 use oci_spec::runtime::Spec;
 
-use crate::sandbox::oci;
+use crate::sandbox::{oci, Stdio};
 
 #[derive(Default)]
 pub struct LinuxContainerExecutor {
-    stdin: Option<RawFd>,
-    stdout: Option<RawFd>,
-    stderr: Option<RawFd>,
+    stdio: Stdio,
     default_executor: DefaultExecutor,
 }
 
 impl LinuxContainerExecutor {
-    pub fn new(stdin: Option<RawFd>, stdout: Option<RawFd>, stderr: Option<RawFd>) -> Self {
+    pub fn new(stdio: Stdio) -> Self {
         Self {
-            stdin,
-            stdout,
-            stderr,
+            stdio,
             ..Default::default()
         }
     }
 }
 
 impl Executor for LinuxContainerExecutor {
     fn exec(&self, spec: &Spec) -> Result<(), ExecutorError> {
-        redirect_io(self.stdin, self.stdout, self.stderr).map_err(|err| {
+        self.stdio.redirect().map_err(|err| {
             log::error!("failed to redirect io: {}", err);
             ExecutorError::Other(format!("failed to redirect io: {}", err))
         })?;
@@ -118,19 +111,3 @@ impl Executor for LinuxContainerExecutor {
         self.default_executor.name()
     }
 }
-
-fn redirect_io(stdin: Option<i32>, stdout: Option<i32>, stderr: Option<i32>) -> anyhow::Result<()> {
-    if let Some(stdin) = stdin {
-        dup(STDIN_FILENO)?;
-        dup2(stdin, STDIN_FILENO)?;
-    }
-    if let Some(stdout) = stdout {
-        dup(STDOUT_FILENO)?;
-        dup2(stdout, STDOUT_FILENO)?;
-    }
-    if let Some(stderr) = stderr {
-        dup(STDERR_FILENO)?;
-        dup2(stderr, STDERR_FILENO)?;
-    }
-    Ok(())
-}

crates/containerd-shim-wasm/src/sandbox/manager.rs

@@ -23,7 +23,7 @@ use super::error::Error;
 use super::instance::Instance;
 use super::{oci, sandbox};
 use crate::services::sandbox_ttrpc::{Manager, ManagerClient};
-use crate::sys::setup_namespaces;
+use crate::sys::networking::setup_namespaces;
 
 /// Sandbox wraps an Instance and is used with the `Service` to manage multiple instances.
 pub trait Sandbox: Task + Send + Sync {

crates/containerd-shim-wasm/src/sandbox/mod.rs

@@ -7,11 +7,13 @@ pub mod instance;
 pub mod instance_utils;
 pub mod manager;
 pub mod shim;
+pub mod stdio;
 
 pub use error::{Error, Result};
 pub use instance::{Instance, InstanceConfig};
 pub use manager::{Sandbox as SandboxService, Service as ManagerService};
 pub use shim::{Cli as ShimCli, Local};
+pub use stdio::Stdio;
 
 pub mod oci;
 

crates/containerd-shim-wasm/src/sandbox/shim.rs

@@ -36,7 +36,8 @@ use ttrpc::context::Context;
 
 use super::instance::{Instance, InstanceConfig, Nop, Wait};
 use super::{oci, Error, SandboxService};
-use crate::sys::{get_metrics, setup_namespaces};
+use crate::sys::metrics::get_metrics;
+use crate::sys::networking::setup_namespaces;
 
 type InstanceDataStatus = (Mutex<Option<(u32, DateTime<Utc>)>>, Condvar);
 

crates/containerd-shim-wasm/src/sandbox/stdio.rs

@@ -0,0 +1,66 @@
+use std::fs::{File, OpenOptions};
+use std::io::ErrorKind::NotFound;
+use std::io::{Error, Result};
+use std::path::Path;
+use std::sync::{Arc, Mutex};
+
+use crate::sys::stdio::*;
+
+#[derive(Default, Clone)]
+pub struct Stdio {
+    pub stdin: Stdin,
+    pub stdout: Stdout,
+    pub stderr: Stderr,
+}
+
+impl Stdio {
+    pub fn redirect(&self) -> Result<()> {
+        self.stdin.redirect()?;
+        self.stdout.redirect()?;
+        self.stderr.redirect()?;
+        Ok(())
+    }
+}
+
+macro_rules! stdio_impl {
+    ( $stdio_type:ident, $fd:expr ) => {
+        #[derive(Default, Clone)]
+        pub struct $stdio_type(Arc<Mutex<Option<File>>>);
+
+        impl<P: AsRef<Path>> TryFrom<Option<P>> for $stdio_type {
+            type Error = std::io::Error;
+            fn try_from(path: Option<P>) -> Result<Self> {
+                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 OpenOptions::new().read(true).write(true).open(path) {
+                        Err(err) if err.kind() == NotFound => Ok(None),
+                        Ok(f) => Ok(Some(f)),
+                        Err(err) => Err(err),
+                    },
+                )
+                .transpose()
+                .map(|opt| Self(Arc::new(Mutex::new(opt.flatten()))))
+            }
+        }
+
+        impl $stdio_type {
+            pub fn redirect(&self) -> Result<()> {
+                if let Some(f) = self.0.try_lock().ok().and_then(|mut f| f.take()) {
+                    let f = try_into_fd(f)?;
+                    let _ = unsafe { libc::dup($fd) };
+                    if unsafe { libc::dup2(f.as_raw_fd(), $fd) } == -1 {
+                        return Err(Error::last_os_error());
+                    }
+                }
+                Ok(())
+            }
+        }
+    };
+}
+
+stdio_impl!(Stdin, STDIN_FILENO);
+stdio_impl!(Stdout, STDOUT_FILENO);
+stdio_impl!(Stderr, STDERR_FILENO);

crates/containerd-shim-wasm/src/sys/unix.rs → crates/containerd-shim-wasm/src/sys/unix/metrics.rs

@@ -1,15 +1,11 @@
-use std::fs::{self, File};
-use std::os::unix::io::AsRawFd;
+use std::fs;
 use std::path::Path;
 
 use anyhow::Result;
 use cgroups_rs::cgroup::get_cgroups_relative_paths_by_pid;
 use cgroups_rs::hierarchies::{self};
 use cgroups_rs::{Cgroup, Subsystem};
-use containerd_shim::error::Error as ShimError;
 use containerd_shim::{self as shim};
-use nix::sched::{setns, unshare, CloneFlags};
-use oci_spec::runtime;
 use protobuf::well_known_types::any::Any;
 use shim::protos::cgroups::metrics::{
     CPUStat, CPUUsage, MemoryEntry, MemoryStat, Metrics, PidsStat, Throttle,
@@ -118,39 +114,3 @@ pub fn get_metrics(pid: u32) -> Result<Any> {
     let metrics = convert_to_any(Box::new(metrics)).map_err(|e| Error::Others(e.to_string()))?;
     Ok(metrics)
 }
-
-pub fn setup_namespaces(spec: &runtime::Spec) -> Result<()> {
-    let namespaces = spec
-        .linux()
-        .as_ref()
-        .unwrap()
-        .namespaces()
-        .as_ref()
-        .unwrap();
-    for ns in namespaces {
-        if ns.typ() == runtime::LinuxNamespaceType::Network {
-            if let Some(p) = ns.path() {
-                let f = File::open(p).map_err(|err| {
-                    ShimError::Other(format!(
-                        "could not open network namespace {}: {}",
-                        p.display(),
-                        err
-                    ))
-                })?;
-                setns(f.as_raw_fd(), CloneFlags::CLONE_NEWNET).map_err(|err| {
-                    ShimError::Other(format!("could not set network namespace: {0}", err))
-                })?;
-            } else {
-                unshare(CloneFlags::CLONE_NEWNET).map_err(|err| {
-                    ShimError::Other(format!("could not unshare network namespace: {0}", err))
-                })?;
-            }
-        }
-    }
-
-    // Keep all mounts changes (such as for the rootfs) private to the shim
-    // This way mounts will automatically be cleaned up when the shim exits.
-    unshare(CloneFlags::CLONE_NEWNS)
-        .map_err(|err| shim::Error::Other(format!("failed to unshare mount namespace: {}", err)))?;
-    Ok(())
-}

crates/containerd-shim-wasm/src/sys/unix/mod.rs

@@ -0,0 +1,3 @@
+pub mod metrics;
+pub mod networking;
+pub mod stdio;

crates/containerd-shim-wasm/src/sys/unix/networking.rs

@@ -0,0 +1,44 @@
+use std::fs::File;
+use std::os::unix::io::AsRawFd;
+
+use anyhow::Result;
+use containerd_shim::error::Error as ShimError;
+use containerd_shim::{self as shim};
+use nix::sched::{setns, unshare, CloneFlags};
+use oci_spec::runtime;
+
+pub fn setup_namespaces(spec: &runtime::Spec) -> Result<()> {
+    let namespaces = spec
+        .linux()
+        .as_ref()
+        .unwrap()
+        .namespaces()
+        .as_ref()
+        .unwrap();
+    for ns in namespaces {
+        if ns.typ() == runtime::LinuxNamespaceType::Network {
+            if let Some(p) = ns.path() {
+                let f = File::open(p).map_err(|err| {
+                    ShimError::Other(format!(
+                        "could not open network namespace {}: {}",
+                        p.display(),
+                        err
+                    ))
+                })?;
+                setns(f.as_raw_fd(), CloneFlags::CLONE_NEWNET).map_err(|err| {
+                    ShimError::Other(format!("could not set network namespace: {0}", err))
+                })?;
+            } else {
+                unshare(CloneFlags::CLONE_NEWNET).map_err(|err| {
+                    ShimError::Other(format!("could not unshare network namespace: {0}", err))
+                })?;
+            }
+        }
+    }
+
+    // Keep all mounts changes (such as for the rootfs) private to the shim
+    // This way mounts will automatically be cleaned up when the shim exits.
+    unshare(CloneFlags::CLONE_NEWNS)
+        .map_err(|err| shim::Error::Other(format!("failed to unshare mount namespace: {}", err)))?;
+    Ok(())
+}

crates/containerd-shim-wasm/src/sys/unix/stdio.rs

@@ -0,0 +1,9 @@
+use std::io::Result;
+pub use std::os::fd::AsRawFd as StdioAsRawFd;
+use std::os::fd::OwnedFd;
+
+pub use libc::{STDERR_FILENO, STDIN_FILENO, STDOUT_FILENO};
+
+pub fn try_into_fd(f: impl Into<OwnedFd>) -> Result<impl StdioAsRawFd> {
+    Ok(f.into())
+}

crates/containerd-shim-wasm/src/sys/windows.rs → crates/containerd-shim-wasm/src/sys/windows/metrics.rs

@@ -14,8 +14,3 @@ pub fn get_metrics(pid: u32) -> Result<Any> {
     let metrics = convert_to_any(Box::new(m)).map_err(|e| Error::Others(e.to_string()))?;
     Ok(metrics)
 }
-
-pub fn setup_namespaces(spec: &runtime::Spec) -> Result<()> {
-    // noop for now
-    Ok(())
-}

crates/containerd-shim-wasm/src/sys/windows/mod.rs

@@ -0,0 +1,3 @@
+pub mod metrics;
+pub mod networking;
+pub mod stdio;

crates/containerd-shim-wasm/src/sys/windows/networking.rs

@@ -0,0 +1,4 @@
+pub fn setup_namespaces(spec: &oci_spec::runtime::Spec) -> anyhow::Result<()> {
+    // noop for now
+    Ok(())
+}

crates/containerd-shim-wasm/src/sys/windows/stdio.rs

@@ -0,0 +1,39 @@
+use std::io::ErrorKind::Other;
+use std::io::{Error, Result};
+use std::os::windows::prelude::{AsRawHandle, IntoRawHandle, OwnedHandle};
+
+use libc::{c_int, close, intptr_t, open_osfhandle, O_APPEND};
+
+type StdioRawFd = libc::c_int;
+
+pub static STDIN_FILENO: StdioRawFd = 0;
+pub static STDOUT_FILENO: StdioRawFd = 1;
+pub static STDERR_FILENO: StdioRawFd = 2;
+
+struct StdioOwnedFd(c_int);
+
+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"));
+    }
+    let _ = handle.into_raw_handle(); // drop ownership of the handle, it's managed by fd now
+    Ok(StdioOwnedFd(fd))
+}
+
+pub trait StdioAsRawFd {
+    fn as_raw_fd(&self) -> c_int;
+}
+
+impl StdioAsRawFd for StdioOwnedFd {
+    fn as_raw_fd(&self) -> c_int {
+        self.0
+    }
+}
+
+impl Drop for StdioOwnedFd {
+    fn drop(&mut self) {
+        unsafe { close(self.0) };
+    }
+}