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

Update to async-std 1.9, change UDP stream implementation #7

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Frando
Copy link

@Frando Frando commented Feb 15, 2021

This updates the async-std dependency from 1.6.2 to 1.9. The update initially broke reading from the UDP socket. A fix is included in this PR.

  • The poll_next method of UdpFramed now stores the socket.recv_from
    future, the old behavior of recreating the future on each poll is no longer supported by async_std.
    See this issue
    for details.
  • I couldn't get storing the future to work with BytesMut because all references or raw pointers seem to be not Send which is needed for it to work across the await. Thus I changed to a vec, which should be fine too, but not sure.

- The poll_next method of UdpFramed needs to store the socket.recv_from
  future, the old behavior of recreating the future on each poll is no
  longer supported.
  See [this issue](async-rs/async-std#955)
  for details.
- I couldn't get the future to work with BytesMut because all references
  or raw pointers seem to be not Send which is needed for it to work
  across the await. Thus I changed to a vec, which should be fine too,
  but not sure.
@Frando
Copy link
Author

Frando commented Feb 15, 2021

Likely the same should be done for the socket.send_to future as well. It didn't break for me, though – likely because sending a single UDP packet is nearly instant and doesn't need to be woken again.

@Frando
Copy link
Author

Frando commented Feb 15, 2021

I changed the poll_flush implementation to also store the future and not recreate it on each poll.

The clippy and rustfmt fails are unrelated AFAICS.

@Frando Frando mentioned this pull request Feb 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant