Merge #149

149: Cleanup minion r=MikailBag a=MikailBag



Co-authored-by: Mikail Bagishov <bagishov.mikail@yandex.ru>

src/minion/src/linux/mod.rs → src/minion/src/linux.rs

@@ -9,7 +9,7 @@ pub use crate::linux::dominion::{DesiredAccess, LinuxDominion};
 use crate::{
     linux::{
         pipe::{LinuxReadPipe, LinuxWritePipe},
-        util::{err_exit, get_last_error, Handle, IgnoreExt, Pid},
+        util::{err_exit, get_last_error, Handle, Pid},
     },
     Backend, ChildProcess, ChildProcessOptions, DominionOptions, DominionPointerOwner, DominionRef,
     InputSpecification, InputSpecificationData, OutputSpecification, OutputSpecificationData,
@@ -22,7 +22,6 @@ use std::{
     fs,
     io::{Read, Write},
     os::unix::{ffi::OsStrExt, io::IntoRawFd},
-    ptr,
     sync::{
         atomic::{AtomicI64, Ordering},
         Arc, Mutex,
@@ -38,7 +37,7 @@ pub struct LinuxChildProcess {
     stdin: Option<Box<dyn Write + Send + Sync>>,
     stdout: Option<Box<dyn Read + Send + Sync>>,
     stderr: Option<Box<dyn Read + Send + Sync>>,
-    //in order to save dominion while CP is alive
+    // Used to save dominion while CP is alive
     _dominion_ref: DominionRef,
 
     pid: Pid,
@@ -115,7 +114,7 @@ impl Drop for LinuxChildProcess {
         if f.is_err() || !f.unwrap() {
             return;
         }
-        self.kill().ignore();
+        self.kill().ok();
         self.wait_for_exit(time::Duration::from_millis(100))
             .unwrap();
     }
@@ -274,28 +273,6 @@ impl Backend for LinuxBackend {
     }
 }
 
-fn empty_signal_handler(
-    _signal_code: libc::c_int,
-    _signal_info: *mut libc::siginfo_t,
-    _ptr: *mut libc::c_void,
-) {
-}
-
-fn fix_sigchild() {
-    unsafe {
-        let sa_ptr: *mut libc::sigaction = util::allocate_heap_variable();
-        let mut sa = &mut *sa_ptr;
-        sa.sa_sigaction = empty_signal_handler as *mut () as usize;
-        libc::sigemptyset(&mut sa.sa_mask as *mut _);
-        libc::sigaddset(&mut sa.sa_mask as *mut _, libc::SIGCHLD);
-        sa.sa_flags = libc::SA_SIGINFO | libc::SA_RESTART;
-        if libc::sigaction(libc::SIGCHLD, sa_ptr, ptr::null_mut()) == -1 {
-            err_exit("sigaction");
-        }
-    }
-}
-
 pub fn setup_execution_manager() -> LinuxBackend {
-    fix_sigchild();
     LinuxBackend {}
 }

src/minion/src/linux/check.rs

@@ -3,7 +3,7 @@ use libc::*;
 /// Validate environment
 ///
 /// If some problems are present, Some(s) with returned, where s is human-readable string
-///describing these problems
+/// describing these problems
 pub fn check() -> Option<String> {
     let uid = unsafe { getuid() };
     if uid != 0 {

src/minion/src/linux/dominion.rs

@@ -94,7 +94,7 @@ impl LinuxDominion {
         })
     }
 
-    pub(crate) unsafe fn exit(&self) -> crate::Result<()> {
+    pub(crate) fn exit(&self) -> crate::Result<()> {
         jail_common::dominion_kill_all(self.zygote_pid)?;
         Ok(())
     }
@@ -127,14 +127,13 @@ impl LinuxDominion {
 }
 
 impl Drop for LinuxDominion {
-    #[allow(unused_must_use)]
     fn drop(&mut self) {
         use std::os::unix::ffi::OsStrExt;
-        // kill all processes
-        unsafe { self.exit() };
-        // remove cgroups
+        // Kill all processes.
+        self.exit().ok();
+        // Remove cgroups.
         for subsys in &["pids", "memory", "cpuacct"] {
-            fs::remove_dir(jail_common::get_path_for_subsystem(subsys, &self.id));
+            fs::remove_dir(jail_common::get_path_for_subsystem(subsys, &self.id)).ok();
         }
 
         let do_umount = |inner_path: &Path| {
@@ -148,7 +147,7 @@ impl Drop for LinuxDominion {
         };
 
         do_umount(Path::new("proc"));
-        fs::remove_dir(&self.options.isolation_root.join(PathBuf::from("proc")));
+        fs::remove_dir(&self.options.isolation_root.join(PathBuf::from("proc"))).ok();
 
         for x in &self.options.exposed_paths {
             do_umount(&x.dest);

src/minion/src/linux/jail_common.rs

@@ -8,10 +8,10 @@ use tiny_nix_ipc::Socket;
 pub(crate) struct JailOptions {
     pub(crate) max_alive_process_count: u32,
     pub(crate) memory_limit: u64,
-    /// specifies total CPU time for whole dominion.
+    /// Specifies total CPU time for whole dominion.
     pub(crate) time_limit: Duration,
     /// Specifies wall-closk time limit for whole dominion.
-    /// Possible value: time_limit * 3
+    /// Possible value: time_limit * 3.
     pub(crate) wall_time_limit: Duration,
     pub(crate) isolation_root: PathBuf,
     pub(crate) exposed_paths: Vec<PathExpositionOptions>,
@@ -66,9 +66,11 @@ pub(crate) enum Query {
     Poll(PollQuery),
 }
 
-pub(crate) unsafe fn dominion_kill_all(zygote_pid: Pid) -> crate::Result<()> {
-    // we will send SIGTERM to zygote
-    // kernel will kill all other processes by itself
-    libc::kill(zygote_pid, libc::SIGTERM);
+pub(crate) fn dominion_kill_all(zygote_pid: Pid) -> crate::Result<()> {
+    // We will send SIGTERM to zygote, and
+    // kernel will kill all other processes by itself.
+    unsafe {
+        libc::kill(zygote_pid, libc::SIGTERM);
+    }
     Ok(())
 }

src/minion/src/linux/util.rs

@@ -1,9 +1,8 @@
 use libc::{self, c_char, c_int, c_void};
 use std::{
     ffi::{CString, OsStr},
-    io, mem,
+    io,
     os::unix::{ffi::OsStrExt, io::RawFd},
-    ptr,
 };
 use tiny_nix_ipc::{self, Socket};
 
@@ -106,15 +105,6 @@ impl IpcSocketExt for Socket {
     }
 }
 
-pub trait IgnoreExt: Sized {
-    #[allow(unused_must_use)]
-    fn ignore(self) {
-        //empty
-    }
-}
-
-impl<T, E> IgnoreExt for Result<T, E> {}
-
 pub fn duplicate_string(arg: &OsStr) -> *mut c_char {
     unsafe {
         let cstr = CString::new(arg.as_bytes()).unwrap();
@@ -152,22 +142,7 @@ impl io::Write for StraceLogger {
     }
 
     fn flush(&mut self) -> io::Result<()> {
-        //empty
+        // empty
         Ok(())
     }
 }
-
-pub fn allocate_memory(num: usize) -> *mut c_char {
-    unsafe {
-        let p = libc::malloc(num) as *mut c_char;
-        if p as usize == 0 {
-            panic!("OutOfMemory: malloc returned null");
-        }
-        ptr::write_bytes(p, 0xDC, num);
-        p
-    }
-}
-
-pub fn allocate_heap_variable<T>() -> *mut T {
-    allocate_memory(mem::size_of::<T>()) as *mut T
-}