From c5950af87d34485b2395e50eb897b7f65f8d248d Mon Sep 17 00:00:00 2001
From: mwrock <matt@mattwrock.com>
Date: Thu, 2 Apr 2020 15:22:03 -0700
Subject: [PATCH] explicitly clear HANDLE_FLAG_INHERIT of cloned sockets on
 windows

Signed-off-by: mwrock <matt@mattwrock.com>
---
 Cargo.lock                         |  1 +
 components/butterfly/Cargo.toml    |  3 +++
 components/butterfly/src/error.rs  |  2 ++
 components/butterfly/src/server.rs | 31 ++++++++++++++++++++++++++++--
 4 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index 24b93581206..350dcd0f643 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -1337,6 +1337,7 @@ dependencies = [
  "threadpool 1.7.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "toml 0.5.6 (registry+https://github.com/rust-lang/crates.io-index)",
  "uuid 0.8.1 (registry+https://github.com/rust-lang/crates.io-index)",
+ "winapi 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)",
  "zmq 0.8.3 (git+https://github.com/habitat-sh/rust-zmq?branch=v0.8-symlinks-removed)",
 ]
 
diff --git a/components/butterfly/Cargo.toml b/components/butterfly/Cargo.toml
index b169c8d870f..aff99d9a76e 100644
--- a/components/butterfly/Cargo.toml
+++ b/components/butterfly/Cargo.toml
@@ -33,6 +33,9 @@ toml = { version = "*", default-features = false }
 uuid = { version = "*", features = ["v4"] }
 zmq = { git = "https://github.com/habitat-sh/rust-zmq", branch = "v0.8-symlinks-removed" }
 
+[target.'cfg(windows)'.dependencies]
+winapi = { version = "*", features = ["handleapi", "winbase"] }
+
 [dev-dependencies]
 mktemp = "*"
 
diff --git a/components/butterfly/src/error.rs b/components/butterfly/src/error.rs
index 6cbd40e2cca..f82b87d7307 100644
--- a/components/butterfly/src/error.rs
+++ b/components/butterfly/src/error.rs
@@ -26,6 +26,7 @@ pub enum Error {
     IncarnationParse(PathBuf, num::ParseIntError),
     InvalidRumorShareLimit,
     NonExistentRumor(String, String),
+    OsError(io::Error),
     ProtocolMismatch(&'static str),
     ServiceConfigDecode(String, toml::de::Error),
     ServiceConfigNotUtf8(String, str::Utf8Error),
@@ -79,6 +80,7 @@ impl fmt::Display for Error {
                 format!("Non existent rumor asked to be written to bytes: {} {}",
                         member_id, rumor_id)
             }
+            Error::OsError(ref err) => format!("OS error: {}", err),
             Error::ProtocolMismatch(ref field) => {
                 format!("Received an unsupported or bad protocol message. Missing field: {}",
                         field)
diff --git a/components/butterfly/src/server.rs b/components/butterfly/src/server.rs
index 6a8a0c86ba8..7e524e7514e 100644
--- a/components/butterfly/src/server.rs
+++ b/components/butterfly/src/server.rs
@@ -501,11 +501,11 @@ impl Server {
         socket.set_write_timeout(Some(Duration::from_millis(1000)))
               .map_err(Error::SocketSetReadTimeout)?;
 
-        self.socket = Some(socket.try_clone()?);
+        self.socket = Some(clone_socket(&socket)?);
 
         inbound::spawn_thread(format!("inbound-{}", self.name()),
                               self.clone(),
-                              socket.try_clone()?,
+                              clone_socket(&socket)?,
                               tx_outbound)?;
 
         outbound::spawn_thread(format!("outbound-{}", self.name()),
@@ -1280,6 +1280,33 @@ fn persist_loop(server: &Server) -> ! {
     }
 }
 
+/// There is a bug which surfaced in rust 1.38 where cloned sockets on windows
+/// get inherited by child processes and remain open even after the process that
+/// created the socket terminates as long as the child processes remain alive.
+/// See https://github.com/rust-lang/rust/issues/70719.
+/// Until this is fixed, we explicitly clear the HANDLE_FLAG_INHERIT of the
+/// socket's handle.
+#[cfg(windows)]
+fn clone_socket(socket: &UdpSocket) -> Result<UdpSocket> {
+    use std::os::windows::io::AsRawSocket;
+    use winapi::um::{handleapi,
+                     winbase,
+                     winnt};
+
+    let cloned = socket.try_clone()?;
+    match unsafe {
+              handleapi::SetHandleInformation(cloned.as_raw_socket() as winnt::HANDLE,
+                                              winbase::HANDLE_FLAG_INHERIT,
+                                              0)
+          } {
+        0 => Err(Error::OsError(io::Error::last_os_error())),
+        _ => Ok(cloned),
+    }
+}
+
+#[cfg(unix)]
+fn clone_socket(socket: &UdpSocket) -> Result<UdpSocket> { socket.try_clone() }
+
 /// This is a proxy struct to represent what information we're writing to the dat file, and
 /// therefore what information gets sent out via the HTTP API. Right now, we're just wrapping the
 /// actual Server struct, but this will give us something we can refactor against without