Should Client (and its Futures) implement Send?

[antoyo]

Hello.
I want to rewrite the http example of my library to use hyper since it is now based on tokio.

Here is my attempt at doing so.

It does not compile. I get the following error:

error[E0277]: the trait bound `futures::Future<Error=hyper::Error, Item=hyper::client::Response> + 'static: std::marker::Send` is not satisfied
   --> examples/http.rs:131:24
    |
131 |                 return connect(http_future, NewGif, &self.stream);
    |                        ^^^^^^^ the trait `std::marker::Send` is not implemented for `futures::Future<Error=hyper::Error, Item=hyper::client::Response> + 'static`
    |
    = note: `futures::Future<Error=hyper::Error, Item=hyper::client::Response> + 'static` cannot be sent between threads safely
    = note: required because of the requirements on the impl of `std::marker::Send` for `std::ptr::Unique<futures::Future<Error=hyper::Error, Item=hyper::client::Response> + 'static>`
    = note: required because it appears within the type `std::boxed::Box<futures::Future<Error=hyper::Error, Item=hyper::client::Response> + 'static>`
    = note: required because it appears within the type `hyper::client::FutureResponse`
    = note: required because it appears within the type `futures::MapErr<hyper::client::FutureResponse, [closure@examples/http.rs:158:29: 158:35]>`
    = note: required because it appears within the type `futures::future::chain::Chain<futures::MapErr<hyper::client::FutureResponse, [closure@examples/http.rs:158:29: 158:35]>, futures::stream::Fold<futures::stream::MapErr<hyper::Body, [closure@examples/http.rs:159:28: 159:34]>, [closure@examples/http.rs:159:49: 162:10], std::result::Result<std::vec::Vec<u8>, ()>, std::vec::Vec<u8>>, [closure@examples/http.rs:158:46: 163:6]>`
    = note: required because it appears within the type `futures::AndThen<futures::MapErr<hyper::client::FutureResponse, [closure@examples/http.rs:158:29: 158:35]>, futures::stream::Fold<futures::stream::MapErr<hyper::Body, [closure@examples/http.rs:159:28: 159:34]>, [closure@examples/http.rs:159:49: 162:10], std::result::Result<std::vec::Vec<u8>, ()>, std::vec::Vec<u8>>, [closure@examples/http.rs:158:46: 163:6]>`
    = note: required because it appears within the type `futures::MapErr<futures::AndThen<futures::MapErr<hyper::client::FutureResponse, [closure@examples/http.rs:158:29: 158:35]>, futures::stream::Fold<futures::stream::MapErr<hyper::Body, [closure@examples/http.rs:159:28: 159:34]>, [closure@examples/http.rs:159:49: 162:10], std::result::Result<std::vec::Vec<u8>, ()>, std::vec::Vec<u8>>, [closure@examples/http.rs:158:46: 163:6]>, [closure@examples/http.rs:164:18: 164:24]>`
    = note: required because it appears within the type `impl futures::Future`
    = note: required by `relm::connect`

error[E0277]: the trait bound `futures::Future<Error=hyper::Error, Item=hyper::client::Response> + 'static: std::marker::Send` is not satisfied
   --> examples/http.rs:138:24
    |
138 |                 return connect(http_future, NewImage, &self.stream);
    |                        ^^^^^^^ the trait `std::marker::Send` is not implemented for `futures::Future<Error=hyper::Error, Item=hyper::client::Response> + 'static`
    |
    = note: `futures::Future<Error=hyper::Error, Item=hyper::client::Response> + 'static` cannot be sent between threads safely
    = note: required because of the requirements on the impl of `std::marker::Send` for `std::ptr::Unique<futures::Future<Error=hyper::Error, Item=hyper::client::Response> + 'static>`
    = note: required because it appears within the type `std::boxed::Box<futures::Future<Error=hyper::Error, Item=hyper::client::Response> + 'static>`
    = note: required because it appears within the type `hyper::client::FutureResponse`
    = note: required because it appears within the type `futures::MapErr<hyper::client::FutureResponse, [closure@examples/http.rs:158:29: 158:35]>`
    = note: required because it appears within the type `futures::future::chain::Chain<futures::MapErr<hyper::client::FutureResponse, [closure@examples/http.rs:158:29: 158:35]>, futures::stream::Fold<futures::stream::MapErr<hyper::Body, [closure@examples/http.rs:159:28: 159:34]>, [closure@examples/http.rs:159:49: 162:10], std::result::Result<std::vec::Vec<u8>, ()>, std::vec::Vec<u8>>, [closure@examples/http.rs:158:46: 163:6]>`
    = note: required because it appears within the type `futures::AndThen<futures::MapErr<hyper::client::FutureResponse, [closure@examples/http.rs:158:29: 158:35]>, futures::stream::Fold<futures::stream::MapErr<hyper::Body, [closure@examples/http.rs:159:28: 159:34]>, [closure@examples/http.rs:159:49: 162:10], std::result::Result<std::vec::Vec<u8>, ()>, std::vec::Vec<u8>>, [closure@examples/http.rs:158:46: 163:6]>`
    = note: required because it appears within the type `futures::MapErr<futures::AndThen<futures::MapErr<hyper::client::FutureResponse, [closure@examples/http.rs:158:29: 158:35]>, futures::stream::Fold<futures::stream::MapErr<hyper::Body, [closure@examples/http.rs:159:28: 159:34]>, [closure@examples/http.rs:159:49: 162:10], std::result::Result<std::vec::Vec<u8>, ()>, std::vec::Vec<u8>>, [closure@examples/http.rs:158:46: 163:6]>, [closure@examples/http.rs:164:18: 164:24]>`
    = note: required because it appears within the type `impl futures::Future`
    = note: required by `relm::connect`

So I wonder if there is a type that is not Send in hyper.
Or perhaps it is caused by something else.
Could you please help me to make this example working?

By the way, is there a better way to write the http_get function?
I would like to keep the error instead of ignoring it.
And I wonder how to avoid using fold.

Thanks for your help.

[seanmonstar]

The Client in hyper does not implement Send. It currently makes use of some types, like Rc and unsync channel, that prevents it. They could be replaced with Send versions, but it'd be at a performance cost. Is there a reason you're try to send it to another thread?

[antoyo]

I do not send it to another thread.
I think this issue is caused by boxing the future since BoxFuture requires Send.

Edit: It is indeed caused by boxed() since this example fails to compile with the same error:

        let client = Client::new(&handle);
        let future = client.get(url).map_err(|_| ()).and_then(|res| {
            res.body().map_err(|_| ()).fold(vec![], |mut acc, chunk| {
                acc.extend_from_slice(&chunk);
                Ok(acc)
            })
        })
            .map_err(|_| ())
            .boxed();

It is normal behaviour?
I'll try to switch to Box<Future> instead of BoxFuture.

Edit 2: I've fixed my library to use Box<Future> and now it works.
I still wonder if futures was designed this way on purpose.
It seems to me that BoxFuture is a way to return different types of future from one function.
But it is not usable with some tokio-based crates like hyper and twist.

[jimmycuadra]

@antoyo See rust-lang/futures-rs#228

[seanmonstar]

It is possible for hyper to change its implementation of Client, such that sync types are using internally, and this would allow sending those futures to other threads. It does mean paying a performance penalty even for those not using it on multiple threads, but maybe that's worth it?

[algermissen]

Personally, I'd rather have the user of Client in charge to make that decision. Does it make sense to isolate the relevant structures and let the user pick whether to make them Send or not?

[seanmonstar]

After having looked into it, it'd be rather difficult to change. Tokio's ClientProxy uses a RefCell internally, which means it can't be Sync without a Mutex. And since the Client in hyper puts the proxy in an Rc, to become Send, it would require that the proxy is Sync.

[algermissen]

All fine from my POV - as long as you do not make it Send by default :-)

[mitsuhiko]

I'm trying to use hyper with tokio 0.1 now and it's basically impossible despite the shim in the new tokio-core. With this pull request in tokio-core it's now generally possible to tokio::spawn when a core is running (tokio-rs/tokio-core#314). However since all hyper types are not Send you still cannot use tokio::spawn which requires Send.

The only way to spawn without the Send bound is tokio::executor::current_thread which is not shimmed. I'm not sure what the plans are to make hyper work with new tokio, but my guess is that types will have to implement Send. (Also for my API to make any sense I also convert all my errors to Send which now means that I cannot set an hyper error as failure cause without it being Send as well).

[seanmonstar]

Yes, the Client should be changed to be Send. Here's the pieces that would need to be adjusted in order to do so:

• The Client cannot hold a tokio-core::reactor::Handle, as it is not Send. It either needs to be a Remote, or a new-tokio Handle.
• The Connect and B: Stream bounds need to have Send added. It's needed to make FutureResponse send, since it boxes futures that contain them. For those that don't want to be required to use Send, the lower-level Connection API can exist without those requirements.
• The Executor that is currently boxed would need to have Send added to its bounds as well. This probably will go away entirely with futures 0.2, since the executor can be gotten from the Context argument.

[seanmonstar]

This has been done in master, and will be part of 0.12.0.