Skip to content

Tag Windows socket handles so they can be distinguished from file handles in posix namespace. #23580

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

phatchman
Copy link
Contributor

@phatchman phatchman commented Apr 16, 2025

Fixes #14233

To allow reading/writing from separate threads under windows, the socket needs to be opened with the flag WSA_FLAG_OVERLAPPED. If this is not done, then either the reading thread or writing thread will be blocked indefinitely.

However, setting this flag, means that windows.ReadFile and windows.WriteFile (as used in net.Stream), will fail with "The parameter is incorrect" on a socket opened for overlapped IO, unless they pass an OVERLAPPED structure. This would also require creating and waiting on an event for every read/write and handling the IO_PENDING error case, making this option very inefficient.

My solution is to tag socket handles so that the posix layer can determine the difference between file handles and socket handles. It then calls either ReadFile/WriteFile or WSASend/WSARecv based on the type of handle. There is no in-built way to do determine if a handle is a socket in Windows, but according to ntdefs.h:

// Low order two bits of a handle are ignored by the system and available
// for use by application code as tag bits.  The remaining bits are opaque
// and used to store a serial number and table index.

I set the lower 1 bit of the handle if it was created as a socket. File handles and other handles are unaffected by this change.

To enable this change, the ws2_32 SOCKET type is changed to be a HANDLE. The tagging of the handles is done in std.posix so that if someone wants to use the raw windows or ws2_32 calls, they get the normal un-tagged windows behavior. Only handles created through the posix namespace will be tagged as sockets.

In posix, a check is performed in read() and write() to determine if it is a socket and then makes the appropriate call for the type of handle.

Finally in net.Stream it is no longer necessary to special-case the windows reads and writes. These can now be done using the posix functions.

This also enables windows user to open sockets with posix.socket and read and write to them with posix.read and posix.write, rather than using the net namesspace if they so desire..

The additional benefit is that WSARecv and WSASend support readv/writev semantics. So a future patch can change the posix readv/writev to use WSARecv/WSASend for sockets.

@alexrp
Copy link
Member

alexrp commented Apr 16, 2025

Note that some kernel32.dll APIs make use of these bits, e.g.: https://learn.microsoft.com/en-us/windows/win32/api/ioapiset/nf-ioapiset-getqueuedcompletionstatus

Even if you have passed the function a file handle associated with a completion port and a valid OVERLAPPED structure, an application can prevent completion port notification. This is done by specifying a valid event handle for the hEvent member of the OVERLAPPED structure, and setting its low-order bit. A valid event handle whose low-order bit is set prevents the completion of the overlapped I/O from enqueing a completion packet to the completion port.

You're obviously not going to be assigning a socket handle to hEvent, but there might be other APIs where this could be a problem. Seems worth investigating if we're going to do this.

@phatchman
Copy link
Contributor Author

@alexrp I've gone through every kernel32 API that could conceivably take a file or socket and can't find any other documented instances of those lower bits being used.

If we do find an API, or a new API uses them in the future, it would be simple to mask the lower 2 bits when passing the handle to the API. This could be done in the windows namespace for any API that needs it the masking performed.

If you want to take the super-cautious approach, I could do the tagging only in the posix layer, leaving the windows namespace handles untouched.

Happy to do whatever you think makes the most sense, but I can't see any issues with the current approach. (at least with respect to using the lower 2 bits of the handle)

@phatchman
Copy link
Contributor Author

phatchman commented Apr 17, 2025

On reflection, I think my "conservative" approach is the best one. There is no need for this tagging to done outside of the posix namespace as only posix will use it to determine the type of handle. This removes any remote possibility of issues with using the bottom 2 bits as we know all of the calls that std.posix will make.

I've updated the PR to do the tagging only in std.posix.

P.S. I don't understand why I'm getting the CI build failure on x86_64 for this patch. zig build test and zig test lib\std\std.zig both pass locally on the x86_64-windows-gnu and x86_64-windows-mscv targets. I'm guessing there needs to be a -lws2_32 somewhere in the build for the posix tests. Found the issue.

…ished from file handles. Use socket-specific WSARecv and WSASend instead of ReadFile and WriteFile when reading/writing a socket. Open windows sockets with WSA_FLAG_OVERLAPPED to prevent inifinite blocking when reading/writing a socket from multiple threads.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sockets seemingly can't be read from and written to at the same time on windows
2 participants