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

Implement Unix domain sockets support #52

Closed
wants to merge 6 commits into from
Closed

Implement Unix domain sockets support #52

wants to merge 6 commits into from

Conversation

w1ldptr
Copy link
Contributor

@w1ldptr w1ldptr commented Nov 15, 2014

Basic Unix domain socket stream implementation.
I used modified version of TCP echo server for testing(with socket address in temp dir instead of localhost).
Not sure its good enough to be merged so please comment.

ayosec
ayosec reviewed Nov 15, 2014
View reviewed changes
src/net.rs
match *addr {
UnixAddr(ref path) => {
if path.exists() {
fs::unlink(path).unwrap();
Copy link

Choose a reason for hiding this comment

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

unwrap will panic if the file can't be unlinked. Is it a good practice to use it in a library?

@w1ldptr
Merge fixes for Rust master
2978156
@w1ldptr
Don't check path passed to UnixSocket::bind
a601452 
Leave it up to user to provide non-existent path for bind and provide
MioErrorKind::AddrInUse wrapper to better handle possible EADDRINUSE
error kind from nix-rust
@w1ldptr
Copy link
Contributor Author

w1ldptr commented Nov 19, 2014

Curiously on Linux CI box unit test fails because EPOLLOUT flag is set for bound socket that is waiting to accept connections.
Test passes on my OS X system so it is probably some epoll peculiarity due to the fact that mio "register" function has hardcoded "let interests = EPOLLIN | EPOLLOUT | EPOLLERR | EPOLLRDHUP" and there is no way to have different set of flags for server and client sockets.
I will try to debug it on Ubuntu.

@carllerche
Copy link
Member

Cool, I am looking forward to this.

@w1ldptr
Copy link
Contributor Author

w1ldptr commented Nov 22, 2014

Carl, I see that there is already an issue(#33) to allow specifying interests on EventLoop::register.
Would it be reasonable to first implement support for #33 and then change the bind socket registration to wait for read events only?

@carllerche
Copy link
Member

Hey there. I got all the other pending PRs merged in :)

@carllerche
Copy link
Member

This looks good at a quick glance (it's late here). I will try to go over this closer tomorrow or monday at the latest and hopefully get it merged in!

@carllerche
Copy link
Member

Hi there. I updated the patch to reflect the latest Rust changes and I rebased it into a single commit.

Resolved by ff6d5af

@carllerche carllerche closed this Dec 1, 2014
@carllerche carllerche mentioned this pull request Dec 13, 2014
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@w1ldptr @carllerche @ayosec