Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cloned sockets are inherited by child processes on windows #70719

Open
mwrock opened this issue Apr 2, 2020 · 2 comments
Open

Cloned sockets are inherited by child processes on windows #70719

mwrock opened this issue Apr 2, 2020 · 2 comments
Labels
A-io Area: `std::io`, `std::fs`, `std::net` and `std::path` A-process Area: `std::process` and `std::env` C-bug Category: This is a bug. O-windows Operating system: Windows T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.

Comments

@mwrock
Copy link

mwrock commented Apr 2, 2020

Given this test code running on Windows 10 compiled with rustc 1.41.0:

use std::{net::UdpSocket, os::windows::io::AsRawSocket, process::Command};
use winapi::{
    shared::minwindef::DWORD,
    um::{handleapi, winbase, winnt},
};

fn main() -> std::io::Result<()> {
    let socket1 = UdpSocket::bind("0.0.0.0:9999")?;
    let socket2 = socket1.try_clone()?;

    let mut flags: DWORD = 0;
    unsafe {
        handleapi::GetHandleInformation(socket1.as_raw_socket() as winnt::HANDLE, &mut flags);
    }
    println!(
        "socket 1 HANDLE_FLAG_INHERIT is set: {}",
        ((flags & winbase::HANDLE_FLAG_INHERIT) != 0)
    );

    unsafe {
        handleapi::GetHandleInformation(socket2.as_raw_socket() as winnt::HANDLE, &mut flags);
    }
    println!(
        "socket 2 HANDLE_FLAG_INHERIT is set: {}",
        ((flags & winbase::HANDLE_FLAG_INHERIT) != 0)
    );

    Command::new("notepad.exe").spawn()?;
    Ok(())
}

I would expect UDP port 9999 to no longer be listening after the code terminates and I would expect both socket1 and socket2 to report false on the state of their HANDLE_FLAG_INHERIT.

Instead port 9999 remains in a listening state until I close the spawned child process notepad and socket2 shows that its HANDLE_FLAG_INHERIT is set.

This appears to have been introduced in rust 1.38.0 via PR #60260 which now sets the WSA_FLAG_NO_HANDLE_INHERIT in WSASocketW instead of explicitly clearing the HANDLE_FLAG_INHERIT with SetHandleInformation. Just to confirm, I have compiled this against nightly-2019-07-26-x86_64-pc-windows-msvc (before PR) and nightly-2019-07-27-x86_64-pc-windows-msvc (after PR) and this code behaves as expected before the PR.

On the surface that PR looks solid and I would totally expect it to work. However it seems (surprisingly) that setting the WSA_FLAG_NO_HANDLE_INHERIT flag on a duplicated socket is not effective. I have googled until I could google no longer and alas have found no mention of this behavior.

As a workaround, doing something like:

let in_socket = socket.try_clone()?;
unsafe {
    handleapi::SetHandleInformation(in_socket.as_raw_socket() as winnt::HANDLE, winbase::HANDLE_FLAG_INHERIT, 0)
};

fixes my use case.

Perhaps the correct fix is for duplicate to call set_no_inherit unless it is a UWP app. However I would love it if someone had deeper knowledge as to why this behavior is as it is.

@mwrock mwrock added the C-bug Category: This is a bug. label Apr 2, 2020
@jonas-schievink jonas-schievink added O-windows Operating system: Windows T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. labels Apr 2, 2020
@Amanieu
Copy link
Member

Amanieu commented Apr 5, 2020

cc @retep998

@retep998
Copy link
Member

retep998 commented Apr 7, 2020

This would not be an issue if we had #38227 solved.

@workingjubilee workingjubilee added A-io Area: `std::io`, `std::fs`, `std::net` and `std::path` A-process Area: `std::process` and `std::env` labels Mar 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-io Area: `std::io`, `std::fs`, `std::net` and `std::path` A-process Area: `std::process` and `std::env` C-bug Category: This is a bug. O-windows Operating system: Windows T-libs-api Relevant to the library API team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

5 participants