Skip to content

Backport pr #1388 to v0.6 #1390

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

Closed
Thomasdezeeuw opened this issue Nov 6, 2020 · 7 comments
Closed

Backport pr #1388 to v0.6 #1390

Thomasdezeeuw opened this issue Nov 6, 2020 · 7 comments
Assignees

Comments

@Thomasdezeeuw
Copy link
Collaborator

Pr: #1388
Issue: #1386

@Thomasdezeeuw Thomasdezeeuw added this to the Maybe backport v0.6 milestone Nov 6, 2020
@Thomasdezeeuw Thomasdezeeuw self-assigned this Nov 6, 2020
@Thomasdezeeuw
Copy link
Collaborator Author

Mio 0.6.x itself is fine, but the problem is in the net2 crate: deprecrated/net2-rs#105.

@faern
Copy link
Contributor

faern commented Nov 7, 2020

So when the net2 PR is merged and net2 0.2.36 is released. Let's bump mio so it says it requires at least net2 = 0.2.36? That way people will get it even if they later do cargo update -p mio in their projects.

Would be great if we could also do the same for tokio. Get a tokio 0.2.23 patch release published that forces the use of this newer mio.

My own experience is that selectively upgrading some dependencies is somewhat common. And we would like this patch to propagate as fast as possible.

@Thomasdezeeuw
Copy link
Collaborator Author

So when the net2 PR is merged and net2 0.2.36 is released. Let's bump mio so it says it requires at least net2 = 0.2.36? That way people will get it even if they later do cargo update -p mio in their projects.

Yes that sounds good.

Would be great if we could also do the same for tokio. Get a tokio 0.2.23 patch release published that forces the use of this newer mio.

I think we can do that, but I want some more prs merged before releasing Mio both 0.7.x and 0.6x.

@faern
Copy link
Contributor

faern commented Nov 13, 2020

I'm thinking that maybe mio 0.6 is not completely without guilt. Or I have made a mistake. Because I try to run the mio test suite under my toolchain built on rust-lang/rust#78802. Without patching net2 I get invalid pointer access (to be expected) but if I build it on top of the net2 code in deprecrated/net2-rs#106 I still get test failures on Windows.

For example test_tcp::peek fails on

poll.register(&s, Token(1), Ready::readable(), PollOpt::edge()).unwrap();

with error kind AddrNetAvailable and the message "The requested address is not valid in its context.".

I have not yet been able to figure out why. I'm not at all knowledgeable in the mio codebase really.

@faern
Copy link
Contributor

faern commented Nov 13, 2020

I don't think it's the net2 PR that is faulty. I have added tests to it where it prints out the c::sockaddr struct content as a byte slice. And with the PR code the results are identical to master for both IPv4 and IPv6 🤷

@faern
Copy link
Contributor

faern commented Nov 13, 2020

Ah.. miow also does the same memory layout assumption: https://github.com/yoshuawuyts/miow/blob/c86373961b97eba692374435e93b81871af3f611/src/net.rs#L459-L470

I'll try to get PRs for that up during the weekend.

@faern
Copy link
Contributor

faern commented Dec 3, 2020

This can be closed, since #1405 is merged and published.

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

No branches or pull requests

2 participants