don't inherit handles by buckd on Windows

Summary:
When test runner waits for `buck2` process it hangs because buckd process is still running.

This happens because buckd inherits handles from parent process and wait on the parent process awaits for these handles to be closed.

Example: https://stackoverflow.com/questions/13243807/popen-waiting-for-child-process-even-when-the-immediate-child-has-terminated

`Popen` in Python has `close_fds` argument to solve this:

> On Windows, if close_fds is true then no handles will be inherited by the child process unless explicitly passed in the handle_list element of STARTUPINFO.lpAttributeList, or by standard handle redirection.
https://docs.python.org/3/library/subprocess.html

Rust sets `bInheritHandles` to `TRUE` unconditionally: https://github.com/rust-lang/rust/blob/acb5c16fa8acf7fd3b48fc218881f006577bab1a/library/std/src/sys/windows/process.rs#L327

There are several open issues related to this:
- rust-lang/rust#54760
- rust-lang/rust#38227

Because of that we have to call `CreateProcessW` ourselves.

Implementation is inspired by:
- std library: https://github.com/rust-lang/rust/blob/master/library/std/src/sys/windows/process.rs
- subprocess library: https://github.com/hniksic/rust-subprocess/blob/master/src/win32.rs
- docs for `CreateProcessW`: https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-createprocessw

Reviewed By: stepancheg

Differential Revision: D36663797

fbshipit-source-id: 14431002a763042f9c160b8a7ff7f8a62dd6befa

cli/src/commands/daemon.rs

@@ -15,10 +15,11 @@ use std::{fs::File, io::Write, path::Path, process, time::Duration};
 
 use anyhow::Context as _;
 use buck2_common::memory;
-use buck2_core::env_helper::EnvHelper;
+use buck2_core::{
+    env_helper::EnvHelper,
+    fs::paths::{AbsPath, ForwardRelativePath},
+};
 use cli_proto::DaemonProcessInfo;
-#[cfg(unix)]
-use daemonize::Daemonize;
 use dice::cycles::DetectCycles;
 use futures::{
     channel::mpsc::{self, UnboundedSender},
@@ -241,9 +242,13 @@ impl DaemonCommand {
     pub fn exec(self, _matches: &clap::ArgMatches, ctx: CommandContext) -> anyhow::Result<()> {
         let project_root = ctx.paths()?.project_root();
         let daemon_dir = ctx.paths()?.daemon_dir()?;
-        let stdout_path = daemon_dir.join("buckd.stdout");
-        let stderr_path = daemon_dir.join("buckd.stderr");
-        let pid_path = daemon_dir.join("buckd.pid");
+        let stdout_path = daemon_dir.join_unnormalized(ForwardRelativePath::new("buckd.stdout")?);
+        let stderr_path = daemon_dir.join_unnormalized(ForwardRelativePath::new("buckd.stderr")?);
+        let pid_path = daemon_dir.join_unnormalized(ForwardRelativePath::new("buckd.pid")?);
+
+        if !daemon_dir.is_dir() {
+            std::fs::create_dir_all(daemon_dir)?;
+        }
         let stdout = File::create(stdout_path)?;
         let stderr = File::create(stderr_path)?;
 
@@ -253,33 +258,9 @@ impl DaemonCommand {
             std::env::set_current_dir(project_root)?;
             self.redirect_output(stdout, stderr)?;
         } else {
-            #[cfg(unix)]
-            {
-                // TODO(cjhopman): Daemonize is pretty un-maintained. We may need to move
-                // to something else or just do it ourselves.
-                let daemonize = Daemonize::new()
-                    .pid_file(pid_path)
-                    .chown_pid_file(true)
-                    .working_directory(project_root)
-                    // This umask corresponds to a default of `rwxr-xr-x` (which is the default on Linux).
-                    // We should probably just leave this alone, but the Daemonize crate doesn't let us do
-                    // that.
-                    .umask(0o022)
-                    .stdout(stdout)
-                    .stderr(stderr);
-                daemonize.start()?;
-            }
-            #[cfg(windows)]
-            {
-                use std::os::windows::process::CommandExt;
-                // Restart current process in detached mode with '--dont-daemonize' flag.
-                let mut cmd = std::process::Command::new(
-                    std::env::current_exe().expect("somehow couldn't get current exe"),
-                );
-                cmd.creation_flags(winapi::um::winbase::DETACHED_PROCESS);
-                cmd.args(std::env::args().skip(1));
-                cmd.arg("--dont-daemonize");
-                cmd.spawn()?;
+            self.daemonize(project_root, &pid_path, stdout, stderr)?;
+            // On Windows process will be restarted.
+            if cfg!(windows) {
                 return Ok(());
             }
         }
@@ -316,4 +297,160 @@ impl DaemonCommand {
         }
         Ok(())
     }
+
+    #[cfg(unix)]
+    fn daemonize(
+        &self,
+        project_root: &AbsPath,
+        pid_path: &AbsPath,
+        stdout: File,
+        stderr: File,
+    ) -> anyhow::Result<()> {
+        // TODO(cjhopman): Daemonize is pretty un-maintained. We may need to move
+        // to something else or just do it ourselves.
+        let daemonize = daemonize::Daemonize::new()
+            .pid_file(pid_path)
+            .chown_pid_file(true)
+            .working_directory(project_root)
+            // This umask corresponds to a default of `rwxr-xr-x` (which is the default on Linux).
+            // We should probably just leave this alone, but the Daemonize crate doesn't let us do
+            // that.
+            .umask(0o022)
+            .stdout(stdout)
+            .stderr(stderr);
+        daemonize.start()?;
+        Ok(())
+    }
+
+    #[cfg(windows)]
+    /// Restart current process in detached mode with '--dont-daemonize' flag.
+    fn daemonize(
+        &self,
+        project_root: &AbsPath,
+        _pid_path: &AbsPath,
+        _stdout: File,
+        _stderr: File,
+    ) -> anyhow::Result<()> {
+        // We have to call CreateProcessW manually because std::process::Command
+        // doesn't allow to set 'bInheritHandles' to false. Without this waiting on
+        // parent process will also wait on inherited handles of daemon process.
+        use std::{
+            env,
+            ffi::{OsStr, OsString},
+            iter, mem,
+            os::windows::ffi::OsStrExt,
+            ptr,
+        };
+
+        use winapi::{
+            shared::minwindef::{DWORD, FALSE},
+            um::{
+                handleapi::CloseHandle,
+                processthreadsapi::{CreateProcessW, PROCESS_INFORMATION, STARTUPINFOW},
+                winbase::{CREATE_NEW_PROCESS_GROUP, CREATE_UNICODE_ENVIRONMENT, DETACHED_PROCESS},
+            },
+        };
+
+        fn to_nullterm(s: &OsStr) -> Vec<u16> {
+            s.encode_wide().chain(iter::once(0)).collect()
+        }
+
+        // Translated from ArgvQuote at http://tinyurl.com/zmgtnls
+        fn append_quoted(arg: &OsStr, cmdline: &mut Vec<u16>) {
+            if !arg.is_empty()
+                && !arg.encode_wide().any(|c| {
+                    c == ' ' as u16
+                        || c == '\t' as u16
+                        || c == '\n' as u16
+                        || c == '\x0b' as u16
+                        || c == '\"' as u16
+                })
+            {
+                cmdline.extend(arg.encode_wide());
+                return;
+            }
+            cmdline.push('"' as u16);
+
+            let arg: Vec<_> = arg.encode_wide().collect();
+            let mut i = 0;
+            while i < arg.len() {
+                let mut num_backslashes = 0;
+                while i < arg.len() && arg[i] == '\\' as u16 {
+                    i += 1;
+                    num_backslashes += 1;
+                }
+
+                if i == arg.len() {
+                    for _ in 0..num_backslashes * 2 {
+                        cmdline.push('\\' as u16);
+                    }
+                    break;
+                } else if arg[i] == b'"' as u16 {
+                    for _ in 0..num_backslashes * 2 + 1 {
+                        cmdline.push('\\' as u16);
+                    }
+                    cmdline.push(arg[i]);
+                } else {
+                    for _ in 0..num_backslashes {
+                        cmdline.push('\\' as u16);
+                    }
+                    cmdline.push(arg[i]);
+                }
+                i += 1;
+            }
+            cmdline.push('"' as u16);
+        }
+
+        #[allow(clippy::vec_init_then_push)]
+        fn make_command_line(program: &OsStr, args: impl Iterator<Item = String>) -> Vec<u16> {
+            let mut cmd: Vec<u16> = Vec::new();
+            cmd.push(b'"' as u16);
+            cmd.extend(program.encode_wide());
+            cmd.push(b'"' as u16);
+            for arg in args.map(OsString::from) {
+                cmd.push(b' ' as u16);
+                append_quoted(&arg, &mut cmd);
+            }
+            cmd.push(0); // null terminator
+            cmd
+        }
+
+        let exe_path = env::current_exe().expect("somehow couldn't get current exe");
+        let program = exe_path.as_os_str();
+        let cwd = project_root.as_os_str();
+
+        let mut cmd = make_command_line(
+            program,
+            env::args()
+                .skip(1)
+                .chain(iter::once("--dont-daemonize".to_owned())),
+        );
+
+        let mut sinfo: STARTUPINFOW = unsafe { mem::zeroed() };
+        sinfo.cb = mem::size_of::<STARTUPINFOW>() as DWORD;
+        let mut pinfo: PROCESS_INFORMATION = unsafe { mem::zeroed() };
+        let creation_flags =
+            CREATE_NEW_PROCESS_GROUP | DETACHED_PROCESS | CREATE_UNICODE_ENVIRONMENT;
+
+        let status = unsafe {
+            CreateProcessW(
+                to_nullterm(program).as_ptr(), // lpApplicationName
+                cmd.as_mut_ptr(),              // lpCommandLine
+                ptr::null_mut(),               // lpProcessAttributes
+                ptr::null_mut(),               // lpThreadAttributes
+                FALSE,                         // bInheritHandles
+                creation_flags,                // dwCreationFlags
+                ptr::null_mut(),               // lpEnvironment
+                to_nullterm(cwd).as_ptr(),     // lpCurrentDirectory
+                &mut sinfo,
+                &mut pinfo,
+            )
+        };
+        if status == 0 {
+            return Err(anyhow::anyhow!(std::io::Error::last_os_error()));
+        }
+        unsafe { CloseHandle(pinfo.hThread) };
+        unsafe { CloseHandle(pinfo.hProcess) };
+        Ok(())
+    }
 }