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.

Sign up for GitHub

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

Jump to bottom

Added try_recv & try_send to UnixDatagram #1677

Merged
merged 2 commits into from
Jul 25, 2020
Merged

Added try_recv & try_send to UnixDatagram #1677

merged 2 commits into from
Jul 25, 2020

Conversation

jean-airoldie
Copy link
Contributor

This allows nonblocking sync send & recv operations on the socket.

Motivation

This is useful for draining the socket's buffer before dropping it. To do so, one would shutdown the writing end of the peer than try_recv until it errors.

@carllerche
Copy link
Member

This seems fine to me. Looks like Ci is failing.

Thoughts @tokio-rs/maintainers ?

@jean-airoldie
Copy link
Contributor Author

jean-airoldie commented Oct 22, 2019 •
edited
Loading

Seems like freebsd doesn't return WouldBlock for some reason. I'll output the specific errorkind to see what we get instead.

@jean-airoldie jean-airoldie force-pushed the into_raw_fd branch 3 times, most recently from 3110a44 to 534224f Compare October 22, 2019 18:52
@jean-airoldie
Copy link
Contributor Author

jean-airoldie commented Oct 22, 2019 •
edited
Loading

Turns out that freebsd &macos return Os { code: 55, kind: Other, message: "No buffer space available" }' instead of WouldBlock when the datagram queue is full.

@Ralith
Copy link
Contributor

Ralith commented Oct 22, 2019

This is useful for draining the socket's buffer before dropping it.

When is that needed?

@jean-airoldie
Copy link
Contributor Author

jean-airoldie commented Oct 22, 2019 •
edited
Loading

When is that needed?

I'm passing serialized pointers to shared memory objects so I need to properly deallocate them when dropping the channel.

kleimkuhler
kleimkuhler approved these changes Oct 23, 2019
View reviewed changes
Copy link
Contributor

@kleimkuhler kleimkuhler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good 👍

jean-airoldie added a commit to jean-airoldie/tokio that referenced this pull request Oct 25, 2019
@jean-airoldie
Add UnixDatagram::{try_send_to, try_recv_from}
65fb5f9 
This allows the user to send_to or recv_from from the socket
without blocking.

This is a continuation of tokio-rs#1677.
@jean-airoldie jean-airoldie mentioned this pull request Oct 25, 2019
Closed
jean-airoldie added a commit to jean-airoldie/tokio that referenced this pull request Nov 12, 2019
@jean-airoldie
Add UnixDatagram::{try_send_to, try_recv_from}
d781c84 
This allows the user to send_to or recv_from from the socket
without blocking.

This is a continuation of tokio-rs#1677.
@carllerche
Copy link
Member

I'm good w/ this. Could we fix the merge conflicts and I would ❤️ if the API docs could be padded out a bit and have an example.

@jean-airoldie
Copy link
Contributor Author

Almost forgot about this PR.

I added the try_send_to and try_recv_from methods as well. I improved the documentation and added 2 basics tests in the doc to show usage.

@jean-airoldie jean-airoldie mentioned this pull request Dec 23, 2019
Closed
@kleimkuhler
Copy link
Contributor

@jean-airoldie Sorry there has not been any new activity on this. It should still be good to go, but it looks like rusfmt is failing. The build was cleared, but it looks like try_send_to needs a comma after P: AsRef<Path>.

Once the build is passing again it should be 👍

@jean-airoldie
Copy link
Contributor Author

I'll rebase against master to see if this fixes it. If I run rustfmt on my machine I see nothing wrong.

@jean-airoldie
Added try_recv{,from} & try_send{,to} to UnixDatagram
ac6c91c 
This allows nonblocking sync send & recv operations on the socket.
@jean-airoldie
Copy link
Contributor Author

I manually edited it. Its weird that I doesn't show up on my machine because I'm using the same rustfmt version than the CI.

@kleimkuhler
Copy link
Contributor

@jean-airoldie When manually checking formatting I usually do cargo fmt --all -- --check, which on your branch before the previous commit did not show any errors. If that is how you were checking, I don't think an error shows up because the datagram.rs file is not included in the default features.

CI seems to address this by passes all the files manually with a find command.

@kleimkuhler kleimkuhler requested a review from carllerche January 9, 2020 19:02
@Darksonn
Copy link
Contributor

This would also make sense to add to the split UnixDatagram. However, I don't think it should block merging this PR.

@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
@Darksonn Darksonn merged commit 2d97d5a into tokio-rs:master Jul 25, 2020
@Darksonn Darksonn mentioned this pull request Jul 25, 2020
Closed
@Darksonn Darksonn mentioned this pull request Nov 10, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kleimkuhler kleimkuhler kleimkuhler approved these changes

@Ralith Ralith Ralith approved these changes

@carllerche carllerche carllerche approved these changes

Assignees
No one assigned
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
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jean-airoldie @carllerche @Ralith @kleimkuhler @Darksonn