Skip to content

Commit

Permalink
Merge pull request #7597 from habitat-sh/socket
Browse files Browse the repository at this point in the history
fix UDP port remaining open after supervisor terminates when a hook is still executing
  • Loading branch information
mwrock authored Apr 3, 2020
2 parents de23e2e + 83ec312 commit a3573bf
Show file tree
Hide file tree
Showing 5 changed files with 36 additions and 3 deletions.
2 changes: 1 addition & 1 deletion .expeditor/scripts/verify/shared.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ function Initialize-Environment {
$env:PATH = New-PathString -StartingPath $env:PATH -Path "$protobufDir\bin;$zeromqDir\bin;$libarchiveDir\bin;$libsodiumDir\bin;$zlibDir\bin;$xzDir\bin;$opensslDir\bin"

$vsDir = & hab pkg path core/visual-cpp-build-tools-2015
$env:LIB = (Get-Content "$vsDir\LIB_DIRS")
$env:LIB = "$(Get-Content "$vsDir\LIB_DIRS");$env:LIBZMQ_PREFIX\lib"
$env:INCLUDE = (Get-Content "$vsDir\INCLUDE_DIRS")
$env:PATH = New-PathString -StartingPath $env:PATH -Path (Get-Content "$vsDir\PATH")
}
Expand Down
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions components/butterfly/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 = "*"

Expand Down
2 changes: 2 additions & 0 deletions components/butterfly/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down Expand Up @@ -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)
Expand Down
31 changes: 29 additions & 2 deletions components/butterfly/src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()),
Expand Down Expand Up @@ -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) -> std::io::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
Expand Down

0 comments on commit a3573bf

Please sign in to comment.