Using hyper on non-Send Executors

[glommer]

Hello Dear Friends,

I am interested in using your beautiful project in conjunction with https://github.com/DataDog/glommio/ instead of Tokio.
I know there are plenty of issues on how to use hyper with alternative executors, and I got that working just fine.

However, because glommio is a thread-per-core engine it requires that data doesn't leave a thread so our data structures are all ?Send.

Reading the code, it seems to me that the Send requirement arises only because the executor will require Send futures.
Indeed, my simple examples work if I do this:

 unsafe impl Send for HyperStream {}
 unsafe impl Send for HyperListener {}

Where HyperStream and HyperListener are our compat versions of the equivalent hyper data structures.

Although that work, I don't feel very safe: it's too easy for someone to, in the future, trust the Send guarantees and use that data cross-thread in a way that would break my code.

Tokio also has a local non-Send Executor so I am wondering if alternative versions of the API where Send is not a requirement would be welcome.

I am willing to do the work to make that happen, but let's discuss it first

[seanmonstar]

Yea, I wouldn't recommend implementing Send on a type that isn't actually safe to send.

What you want to do should be possible, however. Have you seen the single threaded example: https://github.com/hyperium/hyper/blob/0.13.x/examples/single_threaded.rs

[glommer]

I have, which is what inspired me to undertake this. Prior to that I was thinking it would just not be possible.

The single threaded example passes an Rc to the service fn which is definitely encouraging. But it only overrides the executor.

Let me share with you my snippet. I am creating a listener like this:

        let server = Builder::new(compat::HyperListener(listener), http)
            .executor(compat::HyperExecutor)
            .serve(make_svc);

the HyperListener is a compat library that wraps glommio::Listener, our native, single-threaded Listener type. It accepts a connection and returns a HyperStream, which wraps a glommio::Async<TcpStream>. That's where the problem lies.

If is don't force-mark the TcpStream as Send, I see this:

   --> examples/hyper.rs:109:14
    |
109 |             .serve(make_svc);
    |              ^^^^^ `Rc<glommio::sys::InnerSource>` cannot be sent between threads safely
    |
    = help: within `HyperStream`, the trait `Send` is not implemented for `Rc<glommio::sys::InnerSource>`
    = note: required because it appears within the type `glommio::sys::Source`
    = note: required because it appears within the type `Async<TcpStream>`
    = note: required because it appears within the type `HyperStream`

error[E0277]: `Rc<glommio::sys::InnerSource>` cannot be sent between threads safely
   --> examples/hyper.rs:112:9
    |
112 |         server.await?;
    |         ^^^^^^^^^^^^ `Rc<glommio::sys::InnerSource>` cannot be sent between threads safely
    |
    = help: within `HyperStream`, the trait `Send` is not implemented for `Rc<glommio::sys::InnerSource>`
    = note: required because it appears within the type `glommio::sys::Source`
    = note: required because it appears within the type `Async<TcpStream>`
    = note: required because it appears within the type `HyperStream`
    = note: required because of the requirements on the impl of `Future` for `Server<HyperListener, hyper::service::make::MakeServiceFn<[closure@examples/hyper.rs:104:40: 104:96]>, HyperExecutor>`
    = note: required by `futures_lite::Future::poll`

My type, being non-Send, has an Rc internally and hyper doesn't like that.

Although I am not an expert on hyper (yet?), I have noticed that in the signature of serve, you require the Connection to be Send:

        I::Conn: AsyncRead + AsyncWrite + Unpin + Send + 'static,

Which is why it fails.

I may be missing something, but maybe this check is just too strict? If the types are not checked for Send anywhere in the pipeline, it should still be safe to use on Send executors because the executor would fail to compile, no?

Alternatively what I would propose is a serve_local method that doesn't have the Send requirement.

[seanmonstar]

Ah yes, the Server::builder automatically includes the hyper::upgrade support, which requires the transport to be Send so it can be boxed into an upgraded connection. Notably, the hyper::server::conn::Http API doesn't have that requirement. You skip creating a HyperListener, and simply accept connections in some accept loop and spawn Http::serve_connection futures.

[glommer]

This works like a charm, thanks

[glommer]

One more thing if I may - do you guys plan to publish a similar example to single_threaded.rs for the client-side?
I've been having a lot more trouble with that.

Reading the code I can't find any instance of handshake that doesn't require a Send connection for example, and the impl for Connection itself seems to require Send https://docs.rs/hyper/0.13.9/hyper/client/conn/struct.Connection.html

[seanmonstar]

The client does not have the same flexibility. We'd need to do to the Exec on the client like we've done for the server.

[JanBerktold]

Are there any examples of how to use hyper with gloomio while supporting h1 -> h2 upgrade?

Edit: Added a new issue here: #3624