nim-lang / Nim

Nim is a statically typed compiled systems programming language. It combines successful concepts from mature languages like Python, Ada and Modula. Its design focuses on efficiency, expressiveness, and elegance (in that order of priority).
https://nim-lang.org
Other
16.54k stars 1.47k forks source link

asyncdispatch recv() procs do not respect absence of SafeDisconn flag #4993

Open endragor opened 7 years ago

endragor commented 7 years ago

On *nix systems (haven't checked Windows) recv() and recvInto() procs consider that disconnection happens only when recv syscall errors out. But actually, quoting the man:

If no messages are available to be received and the peer has performed an orderly shutdown, the value 0 is returned.

So they should actually also handle 0 return value as disconnection and error out if SafeDisconn is not present.

dom96 commented 7 years ago

That's not how the SafeDisconn flag was designed to work in the first place. I don't think an error should be raised when an "orderly shutdown" occurs, because that's not an error.

endragor commented 7 years ago

At least explaining the meaning of SafeDisconn in the doc would be nice. My expectation was that recv() without SafeDisconn would never successfully complete without reading any data. Now it turns out SafeDisconn for recv() is somewhat useless, because you have to handle 0-byte read in either case. At the same time send() will always fail on disconnect without SafeDisconn, even if the other side performed an orderly shutdown.

dom96 commented 7 years ago

Now it turns out SafeDisconn for recv() is somewhat useless, because you have to handle 0-byte read in either case.

The SafeDisconn flag is incredibly useful. It ensures that you can handle simply handle the case where recv returns 0 bytes, without worrying about Connection Reset by Peer and other such errors.

endragor commented 7 years ago

What I meant is that there seems to be no use case when you want not to use it. So in the end it's useless to have a flag that has a single reasonable state.

dom96 commented 7 years ago

What about the case when you want to distinguish between an orderly shutdown and an error?

endragor commented 7 years ago

When do you think that happens? Usually how you handle disconnection is bound to the protocol. If the other side disconnected in the middle of conversation, that's a bad thing, regardless of how it disconnected. And if it disconnected after sending/receiving all the data you wanted, it doesn't matter again how it did that (you will not know it actually, because you've likely closed the socket on your side and not calling send()/recv()).

I've looked at Java APIs, both synchronous and async, and they don't seem to make that distinguishment - read() either completes successfully or doesn't. That design makes sense to me, and that means that none of Java applications in the world needed to make the distinguishment.

dom96 commented 7 years ago

In general I try not to abstract the operating system too much if I can help it. The POSIX APIs deemed it appropriate to have separate errors for non-orderly shutdowns and so we do as well.

stale[bot] commented 4 years ago

This issue has been automatically marked as stale because it has not had recent activity. If you think it is still a valid issue, write a comment below; otherwise it will be closed. Thank you for your contributions.