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

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

Open
Komfr opened this issue Jul 10, 2020 · 4 comments
Open

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

Komfr opened this issue Jul 10, 2020 · 4 comments

Comments

@Komfr
Copy link

Komfr commented Jul 10, 2020

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

  • Native implementation will throw TypeError exception because read_nonblock with exception set to false returns nil which gets passed to <<.
  • C extension returns 0 which is indistinguishable from 0 which it returns when EAGAIN error is returned so the caller is unable to differentiate between those two cases.
  • JRuby version seems to be throwing EOFError
@tarcieri
Copy link
Contributor

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

@Komfr
Copy link
Author

Komfr commented Jul 13, 2020

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
Copy link
Contributor

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
Copy link
Member

That makes sense to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants