Hyper: Updating to std::future::Future

[seanmonstar]

With std::future::Future stabilizing in Rust 1.36, it is a goal to upgrade from [email protected] to std::future::Future in the next breaking change ([email protected]).

---

Status:

• [x] Update hyper lib to std::future https://github.com/hyperium/hyper/pull/1836
• [x] Update doc tests to std::future https://github.com/hyperium/hyper/issues/1850
• [x] Update examples to std::future https://github.com/hyperium/hyper/issues/1849
• [ ] Update tests to std::future https://github.com/hyperium/hyper/issues/1848
• [x] Update h2 to std::future https://github.com/hyperium/hyper/issues/1851

[davidbarsky]

I might be missing something, but is there a tracking issue on Tower to support 0.3 futures?

[dekellum]

You say you are just considering it, but would TLS (Thread Local Storage, I presume) play well with Tokio's multi-threaded executor and, for example with conditional calls to tokio_threadpool::blocking in (Body) Streams and Sinks?

[seanmonstar]

That should be fine. When calling poll methods, we'd still need to materialize the Context before doing so, so there'd be no relying on other libraries keeping things on the same thread. It's more just wanting to reduce the noise of having to pass a Context to many methods of proto::h1::Conn that don't always need it, but might poll the IO.

[seanmonstar]

Most of the dependencies needed are updated in tokio's std-future branch. Disabling tokio-threadpool and h2 should hopefully be enough to start a branch here.

It's probably not worth figuring out some TLS Context thingy, and just passing the argument everywhere.

[softprops]

https://blog.rust-lang.org/2019/07/04/Rust-1.36.0.html

[seanmonstar]

1836 is the PR for this, it's pretty close. Most things work (minus h2), and I have some local work to remove much of added unsafety, which I hope to have ready Monday. I'll likely merge that into master with docs and tests disabled, to allow those things to be worked on in parallel.

[seanmonstar]

Master is now tracking 0.13, and #1836 was merged into it. It lacks HTTP2 support, an unsafe pin audit, and all the tests and examples were disabled, but hopefully those can be worked on in parallel.

[jplatte]

Thanks everyone for working on this! I'm a bit confused about one thing though - is the move to std::future::Future intentionally also a move to async / await? I was assuming I could build a library with MSRV 1.36 that's going to be using hyper 0.13, but a bunch of code has already been committed that relies on #![feature(async_await)] instead of only the Future trait. Is it just too much hassle to convert to the new Future first, and start using async / await later? (which should purely be an implementation detail)

[lnicola]

tokio 0.2 uses await AFAIK.

[jplatte]

Thanks. I asked on the tokio Gitter channel; apparently they want to only release 0.2 once async_await is stabilized.

[abonander]

@seanmonstar are there plans to migrate Body to AsyncRead instead of being a Stream<Item = Chunk>? If it uses AsyncRead I can add async implementations to multipart and abandon multipart-async. I'm wondering if I should maintain two different crates anyway as the ecosystem seems to be migrating wholesale to async.

[davidbarsky]

@abonander Might be a tricky tricky to do so for a few reasons:

• Body isn't _really_ a Stream<Item = Chunk>, it's a Payload.
• AsyncRead doesn't—to the best of my knowledge—have the equivalent of Payload::poll_trailers, which is needed H/2 connections. The closest thing seems to be http-body, but that isn't AsyncRead.

Personally, I'd love to have a shared interface/trait at the moment for the reasons you outlined, but I'm not confident _any_ of the shared interfaces—HttpBody or AsyncRead—tick off all the requirements someone might have. Maybe a blanket implementation for either trait could work/bridge some gaps in the interface?

[abonander]

@davidbarsky I mostly meant if there were plans to add the impl even though I used "instead of". I didn't mean removing any other API.

[davidbarsky]

@abonander Ah, sorry about that; I misread what you said. Maybe? I'm not 100% sure how that could be implemented without needing specialization, but I might be missing something!

[abonander]

@davidbarsky how would specialization be necessary? There's only one possible impl, Body isn't generic. Digging through the code it looks like it uses AsyncRead underneath and then switches to producing Bytes in proto::h1::io.

[seanmonstar]

@abonander

Are there plans to migrate Body to AsyncRead instead of being a Stream<Item = Chunk>?
[..]
I mostly meant if there were plans to add the impl even though I used "instead of". I didn't mean removing any other API.

No plans so far... Here's some reasons of the top of my head why not to:

1. The Body cannot freely implement Stream and AsyncRead. In order to provide AsyncRead, it would need to add a slot to cache a Chunk, in case the AsyncRead::poll_read buffer wasn't big enough to copy the full thing. This would make the struct bigger and add another branch to Stream::poll_next.
2. The Stream impl can include more specific errors. I suppose an AsyncRead could just translate those to std::io::ErrorKind::Other, though...

If there are good reasons to consider otherwise, we should open a new specific issue to discuss, ya?

[seanmonstar]

All the original "features" on master now use std::future, so I'm going to close this. Some tests and docs still need updating, thought.