Skip to content

windows: consider skipping WSACleanup on exit #141799

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.

Sign up for GitHub

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

Jump to bottom
Open
jstarks opened this issue May 30, 2025 · 2 comments · May be fixed by #141809
Open

windows: consider skipping WSACleanup on exit #141799

jstarks opened this issue May 30, 2025 · 2 comments · May be fixed by #141809
Labels
A-io Area: `std::io`, `std::fs`, `std::net` and `std::path` C-bug Category: This is a bug. O-windows Operating system: Windows T-libs Relevant to the library team, which will review and decide on the PR/issue.

Comments

@jstarks
Copy link

jstarks commented May 30, 2025

On Windows, std calls WSACleanup at exit if it called WSAStartup, to uninitialize Windows sockets:

rust/library/std/src/sys/net/connection/socket/windows.rs

Lines 137 to 144 in 15825b7

pub fn cleanup() {
// only perform cleanup if network functionality was actually initialized
if let Some(cleanup) = WSA_CLEANUP.get() {
unsafe {
cleanup();
}
}
}

This is an unnecessary expense--the process is going away, so there's nothing WSACleanup can do that won't also be done by the kernel when the process terminates. In a short-lived process, the call takes about 60us if you've only used TCP. If you also have used AF_UNIX and AF_HYPERV, for example, it takes about 400us--this is because it has to unload some DLLs that were dynamically loaded. If some of the data is cold, it has the potential to take much longer. None of this is necessary to "cleanly" terminate a process.

Now, arguably, you might want to call WSACleanup on DLL unload, to ensure you've cleaned everything up. But Rust does not do this today, AFAICT, just as it doesn't drop statics, either on DLL unload or on clean exit from main. So this WSACleanup behavior seems inconsistent.

@ChrisDenton wdyt?
@jstarks jstarks added the C-bug Category: This is a bug. label May 30, 2025
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label May 30, 2025
@ChrisDenton
Copy link
Member

ChrisDenton commented May 31, 2025 •
edited
Loading

Seems reasonable to me. I looked back at the history here and it seems it was added in the initial implementation ~11 years ago and more or less kept ever since.

I'd add that std doesn't really support unloading itself. Not that it isn't possible but we don't have any test coverage for that and doing it soundly is very much on the user, as far as I'm aware.

@ChrisDenton ChrisDenton linked a pull request May 31, 2025 that will close this issue
Open
@ChrisDenton
Copy link
Member

The only potential issue I'm hearing is that it may be flagged by some tools as a bug.

@Noratrieb Noratrieb added O-windows Operating system: Windows T-libs Relevant to the library team, which will review and decide on the PR/issue. A-io Area: `std::io`, `std::fs`, `std::net` and `std::path` and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Jun 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-io Area: `std::io`, `std::fs`, `std::net` and `std::path` C-bug Category: This is a bug. O-windows Operating system: Windows T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Don't call WSACleanup on process exit ChrisDenton/rust
4 participants
@ChrisDenton @jstarks @rustbot @Noratrieb