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

[ghost]

trafficstars

I didn't tag this as bug because it might be me doing something wrong, but I'm at the end of my rope here. Here's my client code:

const builtin = @import("builtin");
const std = @import("std");
const net = std.net;
const os = std.os;
const time = std.time;
const Stream = std.net.Stream;
const Thread = std.Thread;
pub fn main() !void {
    if (builtin.os.tag == .windows) {
        _ = try os.windows.WSAStartup(2, 2);
    }
    const alloc = std.heap.page_allocator;
    const stream = try net.tcpConnectToHost(alloc, "server", 9000);
    _ = try Thread.spawn(.{}, read, .{stream});
    time.sleep(time.ns_per_s);
    _ = try Thread.spawn(.{}, write, .{stream});
    while (true) {}
}
fn read(conn: Stream) !void {
    const reader = conn.reader();
    std.debug.print("trying to read\n", .{});
    _ = try reader.readByte();
    std.debug.print("read\n", .{});
}
fn write(conn: Stream) !void {
    const writer = conn.writer();
    try writer.writeByte(1);
    std.debug.print("wrote\n", .{});
}

And the server:

const std = @import("std");
const Address = std.net.Address;
const Stream = std.net.Stream;
const StreamServer = std.net.StreamServer;
const Thread = std.Thread;
pub fn main() !void {
    var listener = StreamServer.init(.{ .reuse_address = true });
    defer listener.deinit();
    try listener.listen(try Address.resolveIp("::", 9000));
    while (true) {
        const conn = try listener.accept();
        _ = try Thread.spawn(.{}, read, .{conn.stream});
    }
}
fn read(conn: Stream) !void {
    const reader = conn.reader();
    const b = try reader.readByte();
    std.debug.print("read {}\n", .{b});
}

Expected behavior

The client shows "trying to read", then "wrote".

Actual behavior

On Windows, the client shows only "trying to read", never "wrote".

If I change the order of the threads so that the write happens before it starts trying to read, the write goes through. It's as if writes can't be done while another thread is trying to read from the same socket.

On Linux, it works as expected - the write goes through, even while the read is blocked.

I investigated the stdlib source and found that net.Stream's Reader and Writer implementations use generic ReadFile and WriteFile syscalls, rather than socket-specific ones. I thought that might be the problem, so I tried modifying it to use sendto and recvfrom instead, but the behavior was the same.

I tried writing a similar program in Rust, provided below, and the problem didn't happen.

use std::net::TcpStream;
use std::thread;
use std::io::Read;
use std::io::Write;
use std::os::windows::io::{FromRawSocket,IntoRawSocket};
fn main() {
	let stream_fd = TcpStream::connect("server:9000").unwrap().into_raw_socket();
	thread::spawn(move || {
		let mut buf = [0];
		println!("trying to read");
		unsafe { TcpStream::from_raw_socket(stream_fd) }.read(&mut buf).unwrap();
		println!("read");
		loop {}
	}); 
	thread::sleep_ms(1000);
	thread::spawn(move || {
		println!("writing");
		unsafe { TcpStream::from_raw_socket(stream_fd) }.write(&[0]).unwrap();
		println!("wrote");
		loop {}
	});
	loop {}
}

Version

0.10.0, also happens on 0.11.0-dev.1240+24b4e643f

[tgschultz]

I believe this is because the socket is not created with WSA_FLAG_OVERLAPPED, which typically sockets are on Windows:

WSA_FLAG_OVERLAPPED 0x01

Create a socket that supports overlapped I/O operations.

Most sockets should be created with this flag set. Overlapped sockets can utilize WSASend, WSASendTo, WSARecv, WSARecvFrom, and WSAIoctl for overlapped I/O operations, which allow multiple operations to be initiated and in progress simultaneously.

Simply adding the flag is insufficient to fix the problem, however, because Read/WriteFile will then have to use the OVERLAPPED structure and handle the IO_PENDING result with GetOverlappedResult. The current implementation of the Read/WriteFile bindings treat IO_PENDING as unreachable.

So I think @ghost's solution of using sendto and recvfrom instead of the file ops would have worked in combination with WSA_FLAG_OVERLAPPED.

[phatchman]

A combination of sendto/recv from and WSA_FLAG_OVERLAPPED does indeed fix the issue. I'll do some more testing and submit a PR.

Updated client test code below:

const builtin = @import("builtin");
const std = @import("std");
const net = std.net;
const os = std.os;
const time = std.time;
const Stream = std.net.Stream;
const Thread = std.Thread;
pub fn main() !void {
    const alloc = std.heap.page_allocator;
    const stream = try net.tcpConnectToHost(alloc, "localhost", 9000);
    _ = try Thread.spawn(.{}, read, .{stream});
    time.sleep(time.ns_per_s);
    _ = try Thread.spawn(.{}, write, .{stream});
    while (true) {}
}
fn read(conn: Stream) !void {
    const reader = conn.reader();
    std.debug.print("trying to read\n", .{});
    _ = try reader.readByte();
    std.debug.print("read\n", .{});
}
fn write(conn: Stream) !void {
    const writer = conn.writer();
    try writer.writeByte(1);
    std.debug.print("wrote\n", .{});
}

[phatchman]

I've found a couple of approaches to this, looking for opinions.

1. Change windows.ReadFile to properly handle overlapped IO by setting a manual reset event when the optional offset is passed to it. Then on .IO_PENDING call GetOverlappedResult to wait for the IO to complete. In posix.socket() add the WSA_OVERLAPPED_FLAG to the socket options.

pub fn ReadFile(in_hFile: HANDLE, buffer: []u8, offset: ?u64) ReadFileError!usize {
    while (true) {
        const want_read_count: DWORD = @min(@as(DWORD, maxInt(DWORD)), buffer.len);
        var amt_read: DWORD = undefined;
        var overlapped_data: OVERLAPPED = undefined;
        const overlapped: ?*OVERLAPPED = if (offset) |off| blk: {
            overlapped_data = .{
                .Internal = 0,
                .InternalHigh = 0,
                .DUMMYUNIONNAME = .{
                    .DUMMYSTRUCTNAME = .{
                        .Offset = @as(u32, @truncate(off)),
                        .OffsetHigh = @as(u32, @truncate(off >> 32)),
                    },
                },
                .hEvent = CreateEventExW(null, null, CREATE_EVENT_MANUAL_RESET, EVENT_ALL_ACCESS) catch {
                    return error.Unexpected;
                },
            };
            break :blk &overlapped_data;
        } else null;
        defer if (overlapped) |o| {
            if (o.hEvent) |e| {
                CloseHandle(e);
            }
        };
        if (kernel32.ReadFile(in_hFile, buffer.ptr, want_read_count, &amt_read, overlapped) == 0) {
            switch (GetLastError()) {
                .IO_PENDING => return try GetOverlappedResult(in_hFile, overlapped.?, true),
                .OPERATION_ABORTED => continue,
                .BROKEN_PIPE => return 0,
                .HANDLE_EOF => return 0,
                .NETNAME_DELETED => return error.ConnectionResetByPeer,
                .LOCK_VIOLATION => return error.LockViolation,
                else => |err| return unexpectedError(err),
            }
        }
        return amt_read;
    }
}

This still means I have to change net.Stream, but I can leave them as windows.ReadFile and just pass 0 for the offset to trigger the overlapped IO code.

• The downside of this is approach we are creating and destroying an event for every read. And it still wouldn't allow posix.read to work on windows sockets because that path shouldn't trigger overlapped io as it might not be a socket we are reading/writing.
• The advantage is that calling WriteFile with an offset should actually work now.

2. Is to only change the net.Stream code to use WSASend and WSaRecv instead of windows.ReadFile / windows.WriteFile and add the WSA_OVERLAPPED_FLAG to the socket options in posix.socket().

• The advantage is we're not needing an event to for each read and write.
• As above, posix read and write still won't work with windows sockets because windows.ReadFile will always fail with "The parameter is incorrect" due to not supplying an overlapped structure.

3. In the posix layer, I could call something like getsockopt() on any handle passed to ReadFile and if that returns not a socket, call windows.ReadFile otherwise call WSARecv.

• The advantage is posix.read and posix.write would work for windows sockets, and we don't need to special-case the net.Stream class for windows.
• The downside is we're going to be calling getsockopt on every read/write to work out if it is a socket or not.

I've done some searching and there is a GetHandleType that just returns false for sockets, but not just for sockets. So getsockopt was the best method I could find. Unless someone can think of a better solution?

This seems like the right approach to me, as someone on Windows would expect to be able to use posix.read on a handle they created with posix.socket. But the system call to do a handle check on each read/write just doesn't sit right.

[phatchman]

OK I think I have a solution. Buried within ntdef.h is:

// // 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. //

So I'm going to prose we tag any file handle that is opened as a socket with a 1 in the lowest bit. Then we can use that bit to determine whether to use ReadFile of WSARecv in the posix functions and remove the special casing for Windows in the net.Stream struct.