Skip to content

Conversation

LemonBoy
Copy link
Contributor

The overlap between files and sockets is minimal and lumping them
together means supporting only a small subset of the functionalities
provided by the OS.
Moreover the socket and file handles are not always interchangeable: on
Windows one should use Winsock's close() call rather than the one used
for common files.

@alexnask alexnask added enhancement Solving this issue will likely involve adding new logic or components to the codebase. standard library This issue involves writing Zig code for the standard library. labels Nov 16, 2020
@alexnask alexnask added this to the 0.8.0 milestone Nov 16, 2020
@Aransentin
Copy link
Contributor

Since it's separated we can call shutdown on the socket before closing it now. Currently the connection is always destroyed with an RST packet instead of the standard FIN/ACK goodbye, which works but is kinda dirty.

The connection might still not be established if the connection is non-blocking however, and so calling shutdown would be an error. I assume some sort of "connection has actually been established" flag would be necessary to solve that.

@Aransentin
Copy link
Contributor

Aransentin commented Nov 16, 2020

I'd also consider adding functions to Stream to turn on TCP_NODELAY and/or SO_KEEPALIVE on the underlying socket; options which (arguably) most TCP streams should have enabled.

@LemonBoy
Copy link
Contributor Author

Currently the connection is always destroyed with an RST packet instead of the standard FIN/ACK goodbye, which works but is kinda dirty.

I think having a separate shutdown method is preferable as that's a different hangup sequence than the close one, the user may want to use one or the other depending on their requirements.

I'd also consider adding functions to Stream to turn on TCP_NODELAY and/or SO_KEEPALIVE on the underlying socket; options which (arguably) most TCP streams should have enabled.

There are a few options that deserve getters/setters: nodelay, keepalive, linger and probably more.

I think we can start a new ticket to brainstorm what's needed in the net API.

@frmdstryr
Copy link
Contributor

I think we can start a new ticket to brainstorm what's needed in the net API.

Agree... net also need to support timeouts on read/write since it currently cannot prevent DOS attacks.

@LemonBoy
Copy link
Contributor Author

Is Windows and/or Azure being a bunch of hot garbage again? Or did I mess up the version check?

Copy link
Member

@andrewrk andrewrk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I support this change, let's just make sure it's CI green

The overlap between files and sockets is minimal and lumping them
together means supporting only a small subset of the functionalities
provided by the OS.
Moreover the socket and file handles are not always interchangeable: on
Windows one should use Winsock's close() call rather than the one used
for common files.
Avoid errors if the socket enters the TIME_WAIT state and we need to
re-execute this test before the OS releases it.

This problem was not really a problem before since the accept()-ed
socket was never closed on the server-side.
Starting from Windows 10 build 17063.
@andrewrk andrewrk merged commit ec91583 into ziglang:master Jan 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Solving this issue will likely involve adding new logic or components to the codebase. standard library This issue involves writing Zig code for the standard library.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants