Repository navigation
aio: Read all message_length bytes from message response - #189
Conversation
`sock_recv` reads up to the specified amount of bytes, and can in some situations return less than the requested amount. Make sure we read all `message_length` bytes of the response before returning.
| logger.error('premature ending while reading message (%s bytes remaining)', | ||
| remaining_length) | ||
| break | ||
| message += buf |
There was a problem hiding this comment.
Is this the most memory efficient way to make the message?
There was a problem hiding this comment.
If you're referring to the temporary buffer, then we can probably limit the maximum size to read per iteration to avoid potentially storing two full copies of the message. When I was testing the message I was receiving was around 50k so I didn't see that as a problem.
There was a problem hiding this comment.
I believe message.append(buf) might avoid a copy, or maybe using memoryview() might help. Let's just make sure we don't copy the memory every time. We know how much to allocate ahead of time.
There was a problem hiding this comment.
Ah I see, we can swap it for a bytearray instead to avoid the copy on append. However that means we either need to construct a new bytes at the end (another copy), or allow _message() to return either bytearray or bytes. Since json.loads() accepts both bytes and bytearray the latter option should work fine.
There was a problem hiding this comment.
Ok go ahead and do that and then it's good to go.
By making the message buffer a `bytearray` instead of `bytes` we can append to the buffer without having to create a new instance for every iteration. To avoid another copy we return this `bytearray` instead of casting to `bytes`. For consistency all return paths now return `bytearray` instead of `bytes`. This swap should not be an issue since all callers pass this return value to `json.loads` which supports both `bytes` and `bytearray`.
|
馃憤 |
2 participants
sock_recvreads up to the specified amount of bytes, and can in somesituations return less than the requested amount. Make sure we read all
message_lengthbytes of the response before returning.From my testing this fixes #175 which I can somewhat consistently reproduce by spamming
get_treecalls.