implement UdpSocket::poll_recv_from

[m-mueller678]

This fixes #209.

[mcches]

Could you add a test that covers pending and ready poll states?

[m-mueller678]

Another possible issue is recreating the future to lock the mutex on each poll call. This is not the intended way to use the mutex. The tokio documentation states that the future will usually not yield to the runtime if the access is uncontended, but that is explicitly not guaranteed. From a quick look at the Mutex code I could not find any situation where it would consistently yield on the first call. But if such a state is possible, this implementation would be wrong.

[mcches]

Another possible issue is recreating the future to lock the mutex on each poll call. This is not the intended way to use the mutex. The tokio documentation states that the future will usually not yield to the runtime if the access is uncontended, but that is explicitly not guaranteed. From a quick look at the Mutex code I could not find any situation where it would consistently yield on the first call. But if such a state is possible, this implementation would be wrong.

It's worth noting that the Mutex is most certainly a hack. This was written as a naive proof-of-concept way back and needs a lot more attention. I've been thinking about a better setup that more closely mirrors the real kernel implementation. UdpSocket moves to just be a handle that wraps a simulated Fd and you actually reach into the host to read/write from the socket. If nothing is there you register to be woken when something arrives.

This is a much larger effort, but I think its where we need to go for Tcp and Udp in turmoil.

[wngr]

[..] actually reach into the host to read/write from the socket. [..]

[..] This is a much larger effort, but I think its where we need to go for Tcp and Udp in turmoil.

While I'm not 100% clear which design you have in mind, UdpSocket::readable currently requiring a mutable reference makes turmoil's {Tcp,Udp}Socket implementations unusable for certain application designs. Whichever underlying implementation will be chosen, it would be really important to model the existing APIs from tokio::net 1-to-1.

Sorry for the drive-by comment.

[mcches]

[..] actually reach into the host to read/write from the socket. [..]
[..] This is a much larger effort, but I think its where we need to go for Tcp and Udp in turmoil.

While I'm not 100% clear which design you have in mind, UdpSocket::readable currently requiring a mutable reference makes turmoil's {Tcp,Udp}Socket implementations unusable for certain application designs. Whichever underlying implementation will be chosen, it would be really important to model the existing APIs from tokio::net 1-to-1.

Sorry for the drive-by comment.

Thanks for stopping by! Whatever we do here will always ensure the API signatures mirror tokio 1:1.

[rcoh]

this also doesn't match the upstream — I think its critical to maintain precise fidelity. We'll need to keep the data around somewhere to avoid discarding it.

The only hack I can think of offhand is to keep a Vec<u8> or similar inside of of Rx. If there is overflow, write data there and immediately schedule a wakeup.

Ultimately, I think dropping data here is too much of a footgun to merge as is.

[m-mueller678]

Oh, I was under the impression that is what tokio does. Does it just deliver the remaining bytes on the next receive call then? I was sure I had had issues with truncated messages in tokio in the past, but it might not have been UDP. I agree, we should match tokio behavior exactly. There is already a buffer in there, so keeping it for the next call would be straight forward.

[rcoh]

ah! I read a lot of code to get to the bottom of this. This does in fact seem to be the (undocumented from Tokio's side) behavior. The exact semantics are platform dependent and also depend on the type of socket you are connected to. Windows will definitely drop them, it seems like Linux can do either.

I think Turmoil dropping these bytes is the harshest and correct behavior (but maybe we should log as we do it?)

https://linux.die.net/man/2/recvfrom

If a message is too long to fit in the supplied buffer, excess bytes may be discarded depending on the type of socket the message is received from.

[rcoh]

I think the right action here is:

• Capture that this is the behavior in a test
• Log or otherwise communicate to the turmoil user that this happened so they can get to the bottom of the problem more easily