cleanup

MikailBag · MikailBag · commit 39aef003ac28 · 2021-02-09T22:40:57.000+03:00
diff --git a/Cargo.toml b/Cargo.toml
@@ -5,8 +5,6 @@ authors = ["Mikail Bagishov <bagishov.mikail@yandex.ru>"]
 edition = "2018"
 
 [dependencies]
-libc = "0.2.86"
-errno = "0.2.7"
 rand = "0.8.3"
 serde = { version = "1.0.123", features = ["derive"] }
 serde_json = "1.0.62"
@@ -17,7 +15,6 @@ once_cell = "1.5.2"
 futures-util = "0.3.12"
 tokio = { version = "1.2.0", features = ["net"] }
 tracing = "0.1.23"
-itoa = "0.4.7"
 
 [target.'cfg(target_os="linux")'.dependencies]
 tiny-nix-ipc = { git = "https://github.com/myfreeweb/tiny-nix-ipc", features = ["ser_json", "zero_copy"] }
@@ -26,6 +23,7 @@ libc = "0.2.86"
 errno = "0.2.7"
 serde_json = "1.0.62"
 tokio = { version = "1.2.0", features = ["net"] }
+itoa = "0.4.7"
 
 [target.'cfg(target_os="windows")'.dependencies]
 winapi = { version = "0.3.9", features = ["std", "processthreadsapi", "jobapi2", "errhandlingapi",
diff --git a/src/windows/child.rs b/src/windows/child.rs
@@ -1,16 +1,16 @@
 use crate::windows::{
     spawn::{spawn, ChildParams, Stdio},
+    util::OwnedHandle,
     Error, ReadPipe, WindowsSandbox, WritePipe,
 };
-use winapi::um::{handleapi::CloseHandle, winnt::HANDLE};
 
 #[derive(Debug)]
 pub struct WindowsChildProcess {
     /// Handle to winapi Process object
-    child: HANDLE,
+    child: OwnedHandle,
 
     /// Handle to main process of child
-    main_thread: HANDLE,
+    main_thread: OwnedHandle,
 
     /// Stdin
     stdin: Option<WritePipe>,
@@ -20,15 +20,6 @@ pub struct WindowsChildProcess {
     stderr: Option<ReadPipe>,
 }
 
-impl Drop for WindowsChildProcess {
-    fn drop(&mut self) {
-        unsafe {
-            CloseHandle(self.main_thread);
-            CloseHandle(self.child);
-        }
-    }
-}
-
 impl WindowsChildProcess {
     pub(in crate::windows) fn create_process(
         options: crate::ChildProcessOptions<WindowsSandbox>,
@@ -46,8 +37,8 @@ impl WindowsChildProcess {
         )?;
 
         Ok(WindowsChildProcess {
-            child: info.hProcess,
-            main_thread: info.hThread,
+            child: OwnedHandle::new(info.hProcess),
+            main_thread: OwnedHandle::new(info.hThread),
             stdin: child_stdin,
             stdout: child_stdout,
             stderr: child_stderr,
diff --git a/src/windows/pipe.rs b/src/windows/pipe.rs
@@ -1,102 +1,49 @@
-use crate::windows::{Cvt, Error};
+use crate::windows::{util::OwnedHandle, Cvt, Error};
 use std::os::windows::io::{FromRawHandle, IntoRawHandle, RawHandle};
 use winapi::{
     shared::minwindef::TRUE,
-    um::{
-        handleapi::CloseHandle, minwinbase::SECURITY_ATTRIBUTES, namedpipeapi::CreatePipe,
-        winnt::HANDLE,
-    },
+    um::{minwinbase::SECURITY_ATTRIBUTES, namedpipeapi::CreatePipe},
 };
 
 #[derive(Debug)]
 pub struct ReadPipe {
-    handle: HANDLE,
+    handle: OwnedHandle,
 }
 
-unsafe impl Send for ReadPipe {}
-unsafe impl Sync for ReadPipe {}
-
 impl IntoRawHandle for ReadPipe {
     fn into_raw_handle(self) -> RawHandle {
-        let h = self.handle;
-        std::mem::forget(self);
-        h
+        self.handle.into_inner()
     }
 }
 
 impl FromRawHandle for ReadPipe {
     unsafe fn from_raw_handle(handle: RawHandle) -> Self {
-        ReadPipe { handle }
-    }
-}
-
-impl std::io::Read for ReadPipe {
-    fn read(&mut self, mut buf: &mut [u8]) -> std::io::Result<usize> {
-        if buf.len() > i32::max_value() as usize {
-            buf = &mut buf[..(i32::max_value() as usize)]
-        }
-        let mut read_cnt = 0;
-        let res = unsafe {
-            winapi::um::fileapi::ReadFile(
-                self.handle,
-                buf.as_mut_ptr().cast(),
-                buf.len() as u32,
-                &mut read_cnt,
-                std::ptr::null_mut(),
-            )
-        };
-
-        if res == 0 {
-            return Err(std::io::Error::last_os_error());
+        ReadPipe {
+            handle: OwnedHandle::new(handle),
         }
-        Ok(read_cnt as usize)
     }
 }
 
-impl Drop for ReadPipe {
-    fn drop(&mut self) {
-        unsafe {
-            CloseHandle(self.handle);
-        }
+impl std::io::Read for ReadPipe {
+    fn read(&mut self, buf: &mut [u8]) -> std::io::Result<usize> {
+        self.handle.read(buf)
     }
 }
 
 #[derive(Debug)]
 pub struct WritePipe {
-    handle: HANDLE,
+    handle: OwnedHandle,
 }
 
-unsafe impl Send for WritePipe {}
-unsafe impl Sync for WritePipe {}
-
 impl IntoRawHandle for WritePipe {
     fn into_raw_handle(self) -> RawHandle {
-        let h = self.handle;
-        std::mem::forget(self);
-        h
+        self.handle.into_inner()
     }
 }
 
 impl std::io::Write for WritePipe {
-    fn write(&mut self, mut buf: &[u8]) -> std::io::Result<usize> {
-        if buf.len() > (i32::max_value() as usize) {
-            buf = &buf[..(i32::max_value() as usize)];
-        }
-        let mut written_cnt = 0;
-        let res = unsafe {
-            winapi::um::fileapi::WriteFile(
-                self.handle,
-                buf.as_ptr().cast(),
-                buf.len() as u32,
-                &mut written_cnt,
-                std::ptr::null_mut(),
-            )
-        };
-        if res != 0 {
-            Ok(written_cnt as usize)
-        } else {
-            Err(std::io::Error::last_os_error())
-        }
+    fn write(&mut self, buf: &[u8]) -> std::io::Result<usize> {
+        self.handle.write(buf)
     }
 
     fn flush(&mut self) -> std::io::Result<()> {
@@ -105,14 +52,6 @@ impl std::io::Write for WritePipe {
     }
 }
 
-impl Drop for WritePipe {
-    fn drop(&mut self) {
-        unsafe {
-            CloseHandle(self.handle);
-        }
-    }
-}
-
 pub(in crate::windows) enum InheritKind {
     Allow,
 }
@@ -133,5 +72,12 @@ pub(in crate::windows) fn make(inherit: InheritKind) -> Result<(ReadPipe, WriteP
             0,
         ))?;
     }
-    Ok((ReadPipe { handle: read }, WritePipe { handle: write }))
+    Ok((
+        ReadPipe {
+            handle: OwnedHandle::new(read),
+        },
+        WritePipe {
+            handle: OwnedHandle::new(write),
+        },
+    ))
 }
diff --git a/src/windows/sandbox.rs b/src/windows/sandbox.rs
@@ -23,8 +23,8 @@ impl WindowsSandbox {
 impl crate::Sandbox for WindowsSandbox {
     type Error = Error;
 
-    fn id(&self) -> &str {
-        todo!()
+    fn id(&self) -> String {
+        self.id.clone()
     }
 
     fn kill(&self) -> Result<(), Self::Error> {
diff --git a/src/windows/spawn.rs b/src/windows/spawn.rs
@@ -1,6 +1,7 @@
 use crate::{
     windows::{
         pipe::{self, ReadPipe, WritePipe},
+        util::OwnedHandle,
         Cvt, Error, WindowsSandbox,
     },
     InputSpecificationData, OutputSpecificationData,
@@ -17,7 +18,7 @@ use winapi::{
     shared::{minwindef::TRUE, winerror::ERROR_INSUFFICIENT_BUFFER},
     um::{
         errhandlingapi::GetLastError,
-        handleapi::{CloseHandle, INVALID_HANDLE_VALUE},
+        handleapi::INVALID_HANDLE_VALUE,
         minwinbase::SECURITY_ATTRIBUTES,
         processthreadsapi::{
             CreateProcessW, DeleteProcThreadAttributeList, GetCurrentProcess,
@@ -34,46 +35,49 @@ use winapi::{
 };
 
 pub(in crate::windows) struct Stdio {
-    pub stdin: HANDLE,
-    pub stdout: HANDLE,
-    pub stderr: HANDLE,
+    pub stdin: Option<OwnedHandle>,
+    pub stdout: Option<OwnedHandle>,
+    pub stderr: Option<OwnedHandle>,
 }
 
 impl Stdio {
     fn make_input(
         spec: crate::InputSpecificationData,
-    ) -> Result<(HANDLE, Option<WritePipe>), Error> {
+    ) -> Result<(Option<OwnedHandle>, Option<WritePipe>), Error> {
         match spec {
-            InputSpecificationData::Handle(h) => Ok((h as HANDLE, None)),
+            InputSpecificationData::Handle(h) => Ok((Some(OwnedHandle::new(h as HANDLE)), None)),
             InputSpecificationData::Pipe => {
                 let (reader, writer) = pipe::make(pipe::InheritKind::Allow)?;
-                Ok((reader.into_raw_handle(), Some(writer)))
+                Ok((
+                    Some(OwnedHandle::new(reader.into_raw_handle())),
+                    Some(writer),
+                ))
             }
-            InputSpecificationData::Empty => Ok((open_empty_readable_file(), None)),
-            InputSpecificationData::Null => Ok((-1_i32 as usize as HANDLE, None)),
+            InputSpecificationData::Empty => Ok((Some(open_empty_readable_file()), None)),
+            InputSpecificationData::Null => Ok((None, None)),
         }
     }
 
     fn make_output(
         spec: crate::OutputSpecificationData,
-    ) -> Result<(HANDLE, Option<ReadPipe>), Error> {
+    ) -> Result<(Option<OwnedHandle>, Option<ReadPipe>), Error> {
         match spec {
-            OutputSpecificationData::Handle(h) => Ok((h as HANDLE, None)),
+            OutputSpecificationData::Handle(h) => Ok((Some(OwnedHandle::new(h as HANDLE)), None)),
             OutputSpecificationData::Pipe => {
                 let (reader, writer) = pipe::make(pipe::InheritKind::Allow)?;
-                Ok((writer.into_raw_handle(), Some(reader)))
+                Ok((
+                    Some(OwnedHandle::new(writer.into_raw_handle())),
+                    Some(reader),
+                ))
             }
-            OutputSpecificationData::Null => Ok((-1_i32 as usize as HANDLE, None)),
+            OutputSpecificationData::Null => Ok((None, None)),
             OutputSpecificationData::Ignore => {
                 let file = std::fs::File::create("C:\\NUL").map_err(|io_err| Error::Syscall {
                     errno: io_err.raw_os_error().unwrap_or(-1) as u32,
                 })?;
-                let file = file.into_raw_handle();
-                let cloned_file = crate::windows::util::duplicate_with_inheritance(file)?;
-                unsafe {
-                    CloseHandle(file);
-                }
-                Ok((cloned_file, None))
+                let file = OwnedHandle::new(file.into_raw_handle());
+                let cloned_file = file.try_clone_with_inheritance()?;
+                Ok((Some(cloned_file), None))
             }
             OutputSpecificationData::Buffer(sz) => unsafe {
                 let sz = sz.unwrap_or(usize::max_value()) as u64;
@@ -88,8 +92,12 @@ impl Stdio {
                 if mmap.is_null() {
                     Cvt::nonzero(0)?;
                 }
-                let child_side = crate::windows::util::duplicate_with_inheritance(mmap)?;
-                Ok((child_side, Some(ReadPipe::from_raw_handle(mmap))))
+                let mmap = OwnedHandle::new(mmap);
+                let child_side = mmap.try_clone_with_inheritance()?;
+                Ok((
+                    Some(child_side),
+                    Some(ReadPipe::from_raw_handle(mmap.into_inner())),
+                ))
             },
         }
     }
@@ -117,14 +125,17 @@ impl Stdio {
     }
 }
 
-fn open_empty_readable_file() -> HANDLE {
-    static EMPTY_FILE_HANDLE: once_cell::sync::Lazy<usize> = once_cell::sync::Lazy::new(|| {
-        let (reader, writer) =
-            pipe::make(pipe::InheritKind::Allow).expect("failed to create a pipe");
-        drop(writer);
-        reader.into_raw_handle() as usize
-    });
-    *EMPTY_FILE_HANDLE as HANDLE
+fn open_empty_readable_file() -> OwnedHandle {
+    static EMPTY_FILE_HANDLE: once_cell::sync::Lazy<OwnedHandle> =
+        once_cell::sync::Lazy::new(|| {
+            let (reader, writer) =
+                pipe::make(pipe::InheritKind::Allow).expect("failed to create a pipe");
+            drop(writer);
+            OwnedHandle::new(reader.into_raw_handle())
+        });
+    EMPTY_FILE_HANDLE
+        .try_clone()
+        .expect("failed to clone a handle")
 }
 
 pub(in crate::windows) struct ChildParams {
@@ -211,9 +222,15 @@ pub(in crate::windows) fn spawn(
 
         startup_info.StartupInfo.cb = size_of::<STARTUPINFOEXW>() as u32;
         startup_info.StartupInfo.dwFlags = STARTF_USESTDHANDLES;
-        startup_info.StartupInfo.hStdInput = stdio.stdin;
-        startup_info.StartupInfo.hStdOutput = stdio.stdout;
-        startup_info.StartupInfo.hStdError = stdio.stderr;
+
+        let cvt_handle = |h: Option<OwnedHandle>| {
+            h.map(OwnedHandle::into_inner)
+                .unwrap_or(INVALID_HANDLE_VALUE)
+        };
+
+        startup_info.StartupInfo.hStdInput = cvt_handle(stdio.stdin);
+        startup_info.StartupInfo.hStdOutput = cvt_handle(stdio.stdout);
+        startup_info.StartupInfo.hStdError = cvt_handle(stdio.stderr);
         startup_info
     };
     let creation_flags = CREATE_UNICODE_ENVIRONMENT | EXTENDED_STARTUPINFO_PRESENT;
diff --git a/src/windows/util.rs b/src/windows/util.rs
diff --git a/src/windows/wait.rs b/src/windows/wait.rs

Original file line number	Diff line number	Diff line change
`@@ -23,8 +23,8 @@ impl WindowsSandbox {`
`23`	`23`	`impl crate::Sandbox for WindowsSandbox {`
`24`	`24`	`type Error = Error;`
`25`	`25`
`26`		`- fn id(&self) -> &str {`
`27`		`- todo!()`
	`26`	`+ fn id(&self) -> String {`
	`27`	`+ self.id.clone()`
`28`	`28`	`}`
`29`	`29`
`30`	`30`	`fn kill(&self) -> Result<(), Self::Error> {`