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

IO::Socket read - "returns $bytes bytes" vs "returns UP TO $bytes bytes" #26

Closed
bbkr opened this issue Dec 26, 2012 · 4 comments
Closed

Comments

@bbkr
Copy link

bbkr commented Dec 26, 2012

There was socket behavior change between Rakudo 2012.11 and 2012.12.

Previously .read(2048) returned 2048 bytes if available, currently it returns 512 (one chunk).
If chunk is smaller than $limit - for example .read(2) - then previous and current version returns 2 bytes.

So param changed its meaning from "desired amount of bytes" to "no more than bytes". That led to discussion about which one is correct. Spec says

Reads and returns $bytes bytes from the handle

while IRC discussion disagree

<sorear> bbkr: you're supposed to keep calling .read until either it returns zero or you have all the bytes
 <bbkr> sorear, what is the point of length argument then?
 <sorear> bbkr: 256 seems a little buggy but in general, packet size limits will prevent you from getting all the data at once, and Berkeley sockets systems will return partial data rather than wait for future packets
 <sorear> bbkr: allows you to set an upper limit on the amount to read
...
<bbkr> so is it spec bug (the param means upper limit), or rakudo bug ?
 <sorear> bbkr: my hubris is telling me the spec is wrong and sockets should always follow a Berkeley-esque interface

If current spec is correct then it's Rakudo bug.

@supernovus
Copy link

I want the spec behaviour on this. I totally dislike the change! If I ask for 2048 bytes, and there are 2048 bytes to give, I should be given that many, period. In fact, in some of my libraries, I depend on this behaviour. Many binary protocols have a fixed length header, which then contains the length of the "data" packet. Say for instance the header is 8 bytes, I would do a read(8), decode the header, extract the message length, and then do a read($len); I would expect this to do as I mean, and return me a Buf with the contents of the message in it. How it does it behind the scenes, I don't really care, I just want my Buf containing my requested data. Anything less is not doing what the user wants, and therefore is wrong.

@bbkr
Copy link
Author

bbkr commented Dec 29, 2012

I also dislike this Rakudo change. Spec behavior is more "DWIM" for implementing communication protocols.

Fast decision is required here because HTTP::Easy, LWP::Simple and JSON::RPC are broken in Star distribution (why it was released despite massive amount of failed tests?) and MongoDB driver. So either we go Berkley way and fix those modules or high priority Rakudo bug should be filled.

@skids
Copy link
Contributor

skids commented Jan 4, 2013

This has to be looked at from a failure-mode and real-time perspective. If the rest of the data just never arrives, or takes way more time to arrive than reasonable in a given problem domain, then you have to crank back some sort of failure. The POSIX API is built to accommodate this, and in addition it is built to accommodate it in non-threaded environments vis-a-vis EAGAIN/EINTR.

That is not to say that IO.read could not be a simpler interface that gathers data until it has the requested amount and/or knows for a fact that there is no more, but if it is as such, its behavior in failure or latency situations should be well defined, and defined in such a way that we don't end up with everyone writing their own wrappers around .recv when they decide to harden their applications and find that the .read method just really isn't up to snuff for production use. The same goes for most of the functions that read/write in the IO spec -- their error handling is rarely even mentioned.

@bbkr
Copy link
Author

bbkr commented Jan 9, 2013

This was fixed in Rakudo rakudo/rakudo@d2d2c85 commit which restored user friendly behavior described in spec.

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

3 participants