zig icon indicating copy to clipboard operation
zig copied to clipboard

(POSIX only) std.net.Stream.close only closes 'send' side of the socket

Open iacore opened this issue 1 year ago • 1 comments

Zig Version

master

Steps to Reproduce and Observed Behavior

pub const Stream = struct {
    /// Underlying platform-defined type which may or may not be
    /// interchangeable with a file system file descriptor.
    handle: posix.socket_t,

    pub fn close(s: Stream) void {
        switch (native_os) {
            .windows => windows.closesocket(s.handle) catch unreachable,
            else => posix.close(s.handle),
        }
    }

Expected Behavior

    pub fn close(s: Stream) void {
        switch (native_os) {
            .windows => windows.closesocket(s.handle) catch unreachable,
            else => posix.shutdown(s.handle),
        }
    }

iacore avatar Apr 13 '24 13:04 iacore

Just curious, but is this causing undefined behavior for you? Or is this marked a bug because it's not the behavior you think should be used in the std lib?

If I have a stream and call .close(...) on it, I definitely expect it to close the file descriptor (i.e., posix.close(...), but I live in linux mostly and am pretty new to posix so take that as you will.

I wouldn't mind if my underlying socket were to be shutdown on a Stream.close(...), but delegating the responsibility of closing the socket's underlying file descriptor to the user in addition to calling Stream.close(...) seems a bit confusing since I (as an uneducated user) would expect that cleanup to happen in the stream's close function.

I wouldn't expect to do something like:

pub fn main() {
    // assume socketfd has already been initialized 
    var stream = std.net.Stream{ .handle = socketfd };

    // to be honest, I've never actually used the std.net.Stream struct
    // so forgive me if this is not how to idiomatically use it
    defer Stream.close(stream);
    defer posix.close(stream.handle);
}

Just my two cents.

DAlba-sudo avatar Apr 23 '24 20:04 DAlba-sudo

missing Steps to Reproduce and Observed Behavior missing Expected Behavior missing Zig Version

andrewrk avatar Aug 15 '24 07:08 andrewrk