Closed petedmarsh closed 10 months ago
Oh god, this is a scary PR, you on the slack yet?
I'm not on slack atm, but I can reply here or via email until I get round to it.
The benchmark I linked includes a check that what's sent to _data
is the
same which is of some comfort.
It's possible there is some new error condition thst I have missed but I haven't been able to find one from reading the docs.
On Tue, 22 Aug 2023, 14:38 Liam, @.***> wrote:
Oh god, this is a scary PR, you on the slack yet?
— Reply to this email directly, view it on GitHub https://github.com/betcode-org/betfair/pull/535#issuecomment-1688105445, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJDC6UX2RB6V5IKUXSFEP3XWSR35ANCNFSM6AAAAAA3Z2IK2Q . You are receiving this because you authored the thread.Message ID: @.***>
@mberk any thoughts on this?
Looks neat
I agree, it's pretty terrifying to be modifying this code. However, the changes logically make sense and the tests - particularly the benchmark - give confidence that everything will continue to work as expected
Some live testing by some brave volunteers would further add to the confidence in the changes
If it helps I have been running this patch myself with no issues
This fixes an admitedly rare-in-practice issue where complete messages get stuck in a buffer but also speeds up the receiving of data from the socket in all cases.
The current socket code waits until the last received data ends with a linefeed. It's not gauarenteed that each messsage sent will be read by a single call to
socket.recv
- the message could be larger than the buffer, or several small messages could be read together. It's possible for complete messages to be stuck in the buffer until a call torecv
returns bytes that end with a linefeed.For the sake of illustrating the issue imagine the buffer is 16 bytes. We can
.recv
a full 16 bytes of data containing one complete and one partial message. (Note it's possible for this to happen even if the amount of received data is smaller than the buffer).This issue is fixed by using socket.makefile as it will split the data received on each linefeed.
self._socket_file
is read-only, it could be made writable too and the places where writes are made to the socket could be changed to use it too, this would work. However, there's no issue reading from the socket usingself._socket_file
only and writing using the raw socket, and there's nothing to be gained by writing viaself._socket_file
so that has not been changed.Benchmark
https://gist.github.com/petedmarsh/0a2775ec706156b892d40a67cb017bef