From 61850f88835bfc71682f9c57c28c7e64080f1389 Mon Sep 17 00:00:00 2001 From: Andrew Kelley Date: Thu, 20 May 2021 12:24:58 -0700 Subject: [PATCH] std: Windows: WSASocketW ensures WSAStartup When WSASocketW gets WSANOTINITIALISED, now it will lock a mutex to safely call WSAStartup and then try again one time. This implementation: * Does not use recursion * Contains a detailed doc comment explaining why things are how they are * Is careful about which errors are surfaced in the respective error sets. `std.os.socket` intentionally does not have "not initialised" as one of the possible errors. --- lib/std/os.zig | 32 +++++++++----------- lib/std/os/windows.zig | 68 +++++++++++++++++++++++++++++++++++------- 2 files changed, 71 insertions(+), 29 deletions(-) diff --git a/lib/std/os.zig b/lib/std/os.zig index d7da67272f..601839ea1c 100644 --- a/lib/std/os.zig +++ b/lib/std/os.zig @@ -2724,29 +2724,25 @@ pub const SocketError = error{ /// The socket type is not supported by the protocol. SocketTypeNotSupported, - - /// The environment for socket control has not been initialised. - NotInitialised, } || UnexpectedError; pub fn socket(domain: u32, socket_type: u32, protocol: u32) SocketError!socket_t { if (builtin.os.tag == .windows) { - // NOTE: windows translates the SOCK_NONBLOCK/SOCK_CLOEXEC flags into windows-analagous operations + // NOTE: windows translates the SOCK_NONBLOCK/SOCK_CLOEXEC flags into + // windows-analagous operations const filtered_sock_type = socket_type & ~@as(u32, SOCK_NONBLOCK | SOCK_CLOEXEC); - const flags: u32 = if ((socket_type & SOCK_CLOEXEC) != 0) windows.ws2_32.WSA_FLAG_NO_HANDLE_INHERIT else 0; - const rc = windows.WSASocketW(@bitCast(i32, domain), @bitCast(i32, filtered_sock_type), @bitCast(i32, protocol), null, 0, flags) catch |err| switch (err) { - error.NotInitialised => again: { - // Before a socket is made Windows requires WSAStartup to be called. - // Let's try doing that now, then make the socket again. If socket creation still fails then there is an underlying issue we cannot solve. - // WSAStartup is supposed to have a pair in the form of WSACleanup to call once all socket operations concluded. - // As of writing that function is never called. - _ = windows.WSAStartup(2, 2) catch { - return error.NotInitialised; - }; - break :again try windows.WSASocketW(@bitCast(i32, domain), @bitCast(i32, filtered_sock_type), @bitCast(i32, protocol), null, 0, flags); - }, - else => return err, - }; + const flags: u32 = if ((socket_type & SOCK_CLOEXEC) != 0) + windows.ws2_32.WSA_FLAG_NO_HANDLE_INHERIT + else + 0; + const rc = try windows.WSASocketW( + @bitCast(i32, domain), + @bitCast(i32, filtered_sock_type), + @bitCast(i32, protocol), + null, + 0, + flags, + ); errdefer windows.closesocket(rc) catch unreachable; if ((socket_type & SOCK_NONBLOCK) != 0) { var mode: c_ulong = 1; // nonblocking diff --git a/lib/std/os/windows.zig b/lib/std/os/windows.zig index db9cc98fdf..9d898dadaf 100644 --- a/lib/std/os/windows.zig +++ b/lib/std/os/windows.zig @@ -1261,7 +1261,7 @@ pub fn WSAStartup(majorVersion: u8, minorVersion: u8) !ws2_32.WSADATA { .WSASYSNOTREADY => return error.SystemNotAvailable, .WSAVERNOTSUPPORTED => return error.VersionNotSupported, .WSAEINPROGRESS => return error.BlockingOperationInProgress, - .WSAEPROCLIM => return error.SystemResources, + .WSAEPROCLIM => return error.ProcessFdQuotaExceeded, else => |err| return unexpectedWSAError(err), }, }; @@ -1280,6 +1280,30 @@ pub fn WSACleanup() !void { }; } +var wsa_startup_mutex: std.Thread.Mutex = .{}; + +/// Microsoft requires WSAStartup to be called to initialize, or else +/// WSASocketW will return WSANOTINITIALISED. +/// Since this is a standard library, we do not have the luxury of +/// putting initialization code anywhere, because we would not want +/// to pay the cost of calling WSAStartup if there ended up being no +/// networking. Also, if Zig code is used as a library, Zig is not in +/// charge of the start code, and we couldn't put in any initialization +/// code even if we wanted to. +/// The documentation for WSAStartup mentions that there must be a +/// matching WSACleanup call. It is not possible for the Zig Standard +/// Library to honor this for the same reason - there is nowhere to put +/// deinitialization code. +/// So, API users of the zig std lib have two options: +/// * (recommended) The simple, cross-platform way: just call `WSASocketW` +/// and don't worry about it. Zig will call WSAStartup() in a thread-safe +/// manner and never deinitialize networking. This is ideal for an +/// application which has the capability to do networking. +/// * The getting-your-hands-dirty way: call `WSAStartup()` before doing +/// networking, so that the error handling code for WSANOTINITIALISED never +/// gets run, which then allows the application or library to call `WSACleanup()`. +/// This could make sense for a library, which has init and deinit +/// functions for the whole library's lifetime. pub fn WSASocketW( af: i32, socket_type: i32, @@ -1288,18 +1312,40 @@ pub fn WSASocketW( g: ws2_32.GROUP, dwFlags: DWORD, ) !ws2_32.SOCKET { - const rc = ws2_32.WSASocketW(af, socket_type, protocol, protocolInfo, g, dwFlags); - if (rc == ws2_32.INVALID_SOCKET) { - switch (ws2_32.WSAGetLastError()) { - .WSAEAFNOSUPPORT => return error.AddressFamilyNotSupported, - .WSAEMFILE => return error.ProcessFdQuotaExceeded, - .WSAENOBUFS => return error.SystemResources, - .WSAEPROTONOSUPPORT => return error.ProtocolNotSupported, - .WSANOTINITIALISED => return error.NotInitialised, - else => |err| return unexpectedWSAError(err), + var first = true; + while (true) { + const rc = ws2_32.WSASocketW(af, socket_type, protocol, protocolInfo, g, dwFlags); + if (rc == ws2_32.INVALID_SOCKET) { + switch (ws2_32.WSAGetLastError()) { + .WSAEAFNOSUPPORT => return error.AddressFamilyNotSupported, + .WSAEMFILE => return error.ProcessFdQuotaExceeded, + .WSAENOBUFS => return error.SystemResources, + .WSAEPROTONOSUPPORT => return error.ProtocolNotSupported, + .WSANOTINITIALISED => { + if (!first) return error.Unexpected; + first = false; + + var held = wsa_startup_mutex.acquire(); + defer held.release(); + + // Here we could use a flag to prevent multiple threads to prevent + // multiple calls to WSAStartup, but it doesn't matter. We're globally + // leaking the resource intentionally, and the mutex already prevents + // data races within the WSAStartup function. + _ = WSAStartup(2, 2) catch |err| switch (err) { + error.SystemNotAvailable => return error.SystemResources, + error.VersionNotSupported => return error.Unexpected, + error.BlockingOperationInProgress => return error.Unexpected, + error.ProcessFdQuotaExceeded => return error.ProcessFdQuotaExceeded, + error.Unexpected => return error.Unexpected, + }; + continue; + }, + else => |err| return unexpectedWSAError(err), + } } + return rc; } - return rc; } pub fn bind(s: ws2_32.SOCKET, name: *const ws2_32.sockaddr, namelen: ws2_32.socklen_t) i32 {