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 std::future #1836

Merged
merged 1 commit into from
Jul 9, 2019
Merged

Update to std::future #1836

merged 1 commit into from
Jul 9, 2019

Conversation

seanmonstar
Copy link
Member

@seanmonstar seanmonstar commented Jun 27, 2019

This is a WIP branch of the update from futures v0.1 to std::future::Future. I'm posting it so the outside can track progress, and allow others to try to contribute. See #1805.

Steps

  • Remove futures v0.1 from dependencies
  • Switch to tokio master that has std::future support
  • Get it compiling (at least the type signatures)
  • Fill in server implementation
    • examples/hello.rs runs and sends responses
    • Figure out "HTTP Upgrade" stuff
  • Fill in client implementation
    • The dispatch channel
    • The Connect trait
    • Update the Pool
    • examples/client.rs runs and prints the body
    • Figure out "HTTP Upgrades"
  • hyper::Body
  • Update h2 support
  • Audit new unsafe usage of Pin 😭

@seanmonstar seanmonstar added this to the 0.13 milestone Jun 27, 2019
Cargo.toml Outdated Show resolved Hide resolved
@Aaron1011
Copy link
Contributor

@seanmonstar: I believe that working with Pin shouldn't actually require any unsafe code.

Looking at the unsafe code you've added, all of it seems to be to support pin projections for custom Future structs that you declare. However, none of these types actually rely on the immovability guarantees provided by Pin - they're just regular structs, which currently work without any of the Pin guarantees.

This means that you should be able to add an unconditional Pin impl for all of these types, and simply call Pin::get_mut on self. Since you don't have any unsafe code that relies on these structs being immovable (e.g. self-referential fields), this will be perfectly safe, and will greatly simply the implementation.

@seanmonstar
Copy link
Member Author

Most things have been updated. Still lacking http2 (hitting those paths will find an unimplemented!). A bit of the unsafety has been removed, but there's still some in here that should get looked at some more.

Master is now tracking 0.13, so I'm going to disable tests and then merge this to master, and file issues to re-enable tests and examples. Hopefully those are smaller chunks that can be parallelized better.

BREAKING CHANGE: All usage of async traits (`Future`, `Stream`,
`AsyncRead`, `AsyncWrite`, etc) are updated to newer versions.
@seanmonstar seanmonstar marked this pull request as ready for review July 9, 2019 22:50
@seanmonstar seanmonstar changed the title WIP: Update to std::future Update to std::future Jul 9, 2019
@seanmonstar seanmonstar merged commit 8f4b05a into master Jul 9, 2019
@seanmonstar seanmonstar deleted the std-future branch July 9, 2019 22:55
@seanmonstar seanmonstar mentioned this pull request Jul 9, 2019
5 tasks
@DoumanAsh
Copy link
Contributor

@seanmonstar JFYI current master cannot be built without runtime feature

@seanmonstar
Copy link
Member Author

@DoumanAsh woops, thanks for letting me know. Master should now be working (though the client doesn't quite have a way to set a custom executor...).

@danieleades
Copy link
Contributor

looks like setting a custom executor requires passing in something that implements tokio_executor::Executor. Should this be changed to futures::task::Spawn?

@seanmonstar
Copy link
Member Author

I think not yet. In fact, both are kinda unstable. I might suggest we just take a closure instead of some 3rd party trait. We can discuss that in its own issue.

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.

5 participants