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

net: Add UdpSocket::{try_send,try_send_to} methods #1979

Merged
merged 7 commits into from Jul 29, 2020

Conversation

kleimkuhler
Copy link
Contributor

Motivation

It is currently not possible send data on a single UdpSocket from multiple
locations because the send methods take &mut self. This change introduces
try_send methods that take &self.

Solution

Closes #1624

The &mut self requirements on send and send_to are enforced so that one
task cannot deadlock another.

If two tasks concurrently register interest in sending data and immediately
sending data would block, one task will overwrite the other's interest. This
will cause the task that "won" the race in registering to deadlock. While this
is an uncommon case, it introduces the possibility of it happening and should be
avoided.

The new try_send and try_send_to methods take &self and sidestep the above
issue by not registering the tasks interest. try_send immediately returns
the number of bytes sent or an error if one occurred. try_send_to first
resolves the given target to a SocketAddr, and then immediately tries sending
the data.

try_recv and try_recv_from

These methods would be equivalent to what is being introduced in this change,
but for receving data on a UdpSocket. This is a less common case, but if it
makes sense to include them in this PR then I can add them.

Signed-off-by: Kevin Leimkuhler kleimkuhler@icloud.com

Signed-off-by: Kevin Leimkuhler <kleimkuhler@icloud.com>
Signed-off-by: Kevin Leimkuhler <kleimkuhler@icloud.com>
@kleimkuhler kleimkuhler self-assigned this Dec 17, 2019
Signed-off-by: Kevin Leimkuhler <kleimkuhler@icloud.com>
@kleimkuhler
Copy link
Contributor Author

Hm this seems a little tricky due to Mio's UdpSocket::{send,recv} methods in v0.6.x.

Before checking if data can even be sent, the binding status of the source is expected to have been registered

This has been changed in v0.7.x and will not be an issue.. Need to figure out what makes sense for this while still on 0.6.

@carllerche
Copy link
Member

carllerche commented Dec 18, 2019 via email

@kleimkuhler
Copy link
Contributor Author

kleimkuhler commented Dec 18, 2019

@carllerche Registration only happens during the first poll_write_ready/poll_read_ready. This is true for Unix as well, however Mio 0.6 does not require Unix resources to be registered when trying to send or receive data.

Edit: This was incorrect. This commet resolves the issue.

@carllerche
Copy link
Member

@kleimkuhler can we change that to register earlier? Maybe in try_send if needed?

Signed-off-by: Kevin Leimkuhler <kleimkuhler@icloud.com>
@kleimkuhler
Copy link
Contributor Author

@carllerche The issue was not registration. I restrucuted the test to work
cross-platform and left the note below on the test.

This test is purposely written such that each time sender sends data on
the socket, receiver awaits the data. On Unix, it would be okay waiting
until the end of the test to receive all the data. On Windows, this would
not be okay because it's resources are completion based (via IOCP). If
data is sent and not yet received, attempting to send more data will result
in ErrorKind::WouldBlock until the first operation completes.

@Mygod
Copy link

Mygod commented May 21, 2020

I am not completely sure what the issue is but it seems like it is because mio does not track how many tasks are concurrently expressing interest for the same object?

@Darksonn Darksonn added A-tokio Area: The main tokio crate C-enhancement Category: A PR with an enhancement or bugfix. M-net Module: tokio/net labels Jul 25, 2020
@carllerche carllerche merged commit 1562bb3 into tokio-rs:master Jul 29, 2020
@Darksonn Darksonn mentioned this pull request Nov 10, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tokio Area: The main tokio crate C-enhancement Category: A PR with an enhancement or bugfix. M-net Module: tokio/net
Projects
None yet
Development

Successfully merging this pull request may close these issues.

udp: enable concurrent use of a single UdpSocket from multiple locations
4 participants