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

Windows socket read blocks if stream closed. #21492

Open
karlseguin opened this issue Sep 23, 2024 · 2 comments
Open

Windows socket read blocks if stream closed. #21492

karlseguin opened this issue Sep 23, 2024 · 2 comments
Labels
bug Observed behavior contradicts documented or intended behavior

Comments

@karlseguin
Copy link
Contributor

Zig Version

0.14.0-dev.1573+4d81e8ee9

Steps to Reproduce and Observed Behavior

If 1 thread is doing a blocking read from a net.Stream when another thread closes the connection, the read blocks. On Linux and MacOS ,the read will unblock and return error.NotOpenForReading.

I believe this should work in Windows, but I'm having a hard time navigating the windows documentation (zig seems to use some deprecated functions, e.g. NtClose) to figure out if this behavior should be supported.

The issue was reported against websocket.zig

This code (cliend and server included), reproduces the issue:

// CLIENT
const std = @import("std");

pub fn main() !void {
    var gpa = std.heap.GeneralPurposeAllocator(.{}){};
    const allocator = gpa.allocator();

    var stream = try std.net.tcpConnectToHost(allocator, "127.0.0.1", 9224);
    const recv_thread = try std.Thread.spawn(.{}, recvThread, .{&stream});
    defer recv_thread.join();

    std.time.sleep(std.time.ns_per_s * 2);

    std.debug.print("closing\n", .{});
    try std.posix.shutdown(stream.handle, .both);
    stream.close();
    std.debug.print("closed\n", .{});
}

fn recvThread(stream: *std.net.Stream) void {
    while (true) {
        var buf: [64]u8 = undefined;
        const n = stream.read(&buf) catch |err| {
            std.debug.print("read err: {}", .{err});
            return;
        };
        if (n == 0) {
            return;
        }
        std.log.info("received: {any}\n", .{buf[0..n]});
    }
}

``

```zig
// SERVER
const std = @import("std");

pub fn main() !void {
    var addr = std.net.Address.initIp4(.{ 127, 0, 0, 1 }, 9224);
    var server = try addr.listen(.{});

    while (true) {
        std.log.info("waiting for connection", .{});
        const connection = try server.accept();
        std.log.info("client connected", .{});
        while (true) {
            var buf: [16]u8 = undefined;
            std.log.info("reading...", .{});
            const len = try connection.stream.read(&buf);
            std.log.info("read result: {d}", .{len});
            if (len == 0) break;
        }
    }
}

/cc @pfgithub

Expected Behavior

Expect read to unblock when socket is shutdown and/or closed.

@karlseguin karlseguin added the bug Observed behavior contradicts documented or intended behavior label Sep 23, 2024
@truemedian
Copy link
Contributor

As far as I can tell, shutdown on windows does not interrupt any running blocking calls, it only prevents new calls and waits for all calls to finish. To be fair, the POSIX definition of shutdown makes no claims about interrupting existing calls either.

Perhaps the thread-to-be-interrupted could use the SO_RCVTIMEO sockopt to occasionally time out and give it a chance to realize it has been shut down.

@pfgithub
Copy link
Contributor

It would basically be spinlocking at that point and would introduce delay on application exit, surely there's a better way? On linux, you can select() on the socket and another pipe that can send a kill command.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Observed behavior contradicts documented or intended behavior
Projects
None yet
Development

No branches or pull requests

3 participants