socketry / nio4r

Cross-platform asynchronous I/O primitives for scalable network clients and servers.
Other
965 stars 86 forks source link

Inconsistent end of stream handling in ByteBuffer::read_from #240

Open Komfr opened 4 years ago

Komfr commented 4 years ago

The three implementations of this method differ in how they handle end of stream:

tarcieri commented 4 years ago

I guess the question here is: what should the behavior be? Throw EOFError? Return nil?

Komfr commented 4 years ago

While I am generally a fan of nil values, I think that EOFError exception would be the most appropriate thing here from the compatibility point of view. First it is already used by one version so robust code using this class is likely to be ready for it. Either explicitly based on knowledge of the implementation or generally by expecting this kind of exception to happen in IO related context. Such code might not need any update to be compatible with the new version unlike the nil result which would require some changes and can cause unexpected bugs if the change is not noticed by its maintainer. Second if the code is not ready, the exception will be easier to notice and fix than nil value which could be propagated to unexpected place and possibly cause some silent issue.

boazsegev commented 4 years ago

IMHO, exceptions should be the exception - associated only with errors... and the EOF situation is not really an error, it's somewhat expected and sometimes even relied upon.

Moreover, exceptions incur a high performance cost, they are implemented by saving the stack and registers of one location and then (when the exception occurs) folding the stack back and "clobbering" and overwriting the CPU registers (some CPU registers actually have their own stack that needs to be reset).

I would suggest nil as the correct return type.

ioquatix commented 3 years ago

That makes sense to me.