samply / beam

🌈 Federated, end-to-end-encrypted, efficient communication among strict network environments.
Apache License 2.0
25 stars 6 forks source link

fix(sockets): correctly handle pending writes to prevent double write #214

Closed Threated closed 3 weeks ago

Threated commented 3 weeks ago

The previous implementation called send on the sink directly without using the SinkWriter adapter to convert it to an AsyncWrite. This meant polling the returned future ourself which seemed to work fine for the majority of cases as it either returned Poll::Ready when we were able to send the item to the underlying io or Poll::Pending when the io was busy matching the expected behavior of the AsyncWrite trait. However there is one other case where the future returned by send might return Poll::Pending which is when it was able to send the data to the io but flushing the underlying io returned Poll::Pending. This is an unfornuate property of the send function which I did not realize when initially implementing it. In pratice this leads to a lot of flushes and in my tests caused the bug that occasionally a packet was sent twice as it has been written to the io but not flushed meansing the future returned Poll::Pending leading to the buffer being sent a second time.

TLDR: The new implementation uses the SinkWriter which both does not flush on every write and ensures data is written properly.