Http::new(core: &'a Core)

[scottlamb]

Currently hyper::server::Server owns its own tokio::reactor::Core. I propose it borrow one instead.

This would allow better sharing the reactor. It'd mean that a Server could share the Core with things that it can't now, such as another hyper server (http + https). Also, it'd mean that the reactor could be available to be used prior to bringing up the hyper server.

For example, I'd like to use it with tokio_signal, which (copying from the example) is intended to be used like this:

let mut core = Core::new().unwrap();
let ctrlc = tokio_signal::ctrl_c(&core.handle());
let stream = core.run(ctrlc).unwrap();

I'd like to install the signal handler before being able to call server.handle() (this would allow me to do things such as flushing async logs prior to dying if a signal is received earlier in the startup process), and I'd like to advance the core myself to turn the future into a stream before starting to serve requests via server.run_until (just seems cleaner in terms of error handling).

Another possible benefit is that it'd make it easier to pass a Remote to the service. The new_service argument to Server::bind now has to be provided before hyper creates the Core. So to pass it the Remote, you have to stuff it later into a std::sync::Mutex that you passed into the lambda or in a lazy_static. That's inconvenient. I think with this way you could simply do something like this:

let core = Core::new();
let remote = core.remote();
let server = hyper::server::Http::new(&core)
    .bind(&addr, move || Ok(MyService(remote.clone())))
    .unwrap();

(Although if services are always intended to be created on a reactor thread, it might be even better for tokio's NewService::new_service to take a Handle, to avoid having to deal with Remote::handle() returning None. That change would make this benefit moot.)

It looks like I can use hyper with a borrowed Core today by avoiding Http::bind in favor of Http::bind_connection. But in doing so, I have to roll my own logic for not only accepting connections but also graceful shutdown, duplicating a lot of hyper/src/server/mod.rs. I'd rather not do that.

[seanmonstar]

The expect evolution is that you could use TcpServer in tokio-proto, and hyper can remove some of it's TCP-specific handling. Half of the work for TcpServer has been merged, the part adding graceful shutdown is still a PR: tokio-rs/tokio-proto#135

I'd probably want to defer those kinds of improvements to the TcpServer itself, since it's not an issue unique to hyper.

cc @alexcrichton

[alexcrichton]

Note that this is also why Hyper's server gives access to its handle and so does tokio-proto's proposed implementation

[radix]

Funny thing that I came to the Hyper issue tracker to search for issues about sharing the Core, to see it at the very top of the list :)

I'm struggling even with using Server::handle, as @alexcrichton suggested -- I think for similar reasons that @scottlamb was talking about, with wanting to set things up before starting the HTTP server. For example, my HTTP server needs to have access to some other reactor-using data, but I can't get the reactor until I start my HTTP server.

I'm still new to both Tokio and Rust, so I don't understand the discussion about TcpServer. It seems that TcpServerInstance also wants to own the Core, which seems similarly problematic. In other asynchronous frameworks I've used, the loop is always something you instantiate separately and pass to things that need it, to facilitate running several different things on the same event loop, so it's surprising to find this inverted structure in the Tokio ecosystem. Am I just misunderstanding this design?

FWIW, the design I'm struggling with is setting up HTTP handlers that will communicate with an "Actor" (I'm trying the unreleased Kabuki library) in order to serialize a certain subset of my operations. The Actor needs to run on the reactor, and so does the Hyper server, but I need to be able to give the actor to my HTTP-handler closure, so I can't set things up in the right order.

[alexcrichton]

@radix keep in mind that these APIs are "intro" apis, they're "easy mode". You can always configure the server yourself more manually by calling the raw methods on tokio-proto that work with connected sockets instead of spinning up a TCP listener for you.

In that sense everything you've mentioned is supported, it just depends on how much logic you'll have to vendor locally one way or another.

You bring up some good points, though, and it may be useful for the server (in tokio-proto and perhaps in Hyper as well) to have a mode of operation where it's constructed from a handle rather than constructing the core itself.

[mwanner]

Coming from a Python Twisted background, I'm also pretty surprised about the protocol implementations creating or owning their event loops. I had a pretty hard time figuring out how the "easy mode" was supposed to work and actually fail to see anything easy, there.

I hacked together something that does work for me, here:
mwanner@ef7c13d
Seeing this discussion, I don't quite this a PR is appropriate, though.

From what I understand, changing hyper to use the TcpServer from tokio_proto should solve this issue. I'd appreciate that.

[seanmonstar]

I had a pretty hard time figuring out how the "easy mode" was supposed to work and actually fail to see anything easy, there.

That's useful feedback, thank you. Were you specifically looking for an 'easy' method to give a Core to? Or you find even the usage of the easy method in the hello world example to hard to figure out?

From what I understand, changing hyper to use the TcpServer from tokio_proto should solve this issue. I'd appreciate that.

So, it currently does, ish. The Http type in hyper implements all the right things to be usable with the TcpServer in tokio-proto, but only tokio-proto master. tokio-proto received some improvements to make integration easier, but those haven't yet been released in a new version to crates.io.

[mwanner]

Thanks for your quick response.

Were you specifically looking for an 'easy' method to give a Core to?

..any method at all. AFAICT the server part insists on creating its own Core. Or am I missing something? That's what I tried to fix with my patch (talking about and patching against master).

Or you find even the usage of the easy method in the hello world example to hard to figure out?

I'm missing the event loop from examples/server.rs. How would I run two HTTP Server instances on the same event loop?

[seanmonstar]

AFAICT the server part insists on creating its own Core. Or am I missing something?

The type hyper::server::Server is basically a specialized version of tokio-proto's TcpServer at the moment. It's created from Http::bind, and is meant for the 'easy' case, which is "start using this thread to handle HTTP connections".

I'm missing the event loop from examples/server.rs. How would I run two HTTP Server instances on the same event loop?

It's true that we're currently light on docs. I expect one "guide" in #805 to discuss the advanced uses of the Http type, such as creating a Core outside of hyper, listening for connections from wherever you want, and just using the Http type to drive the HTTP state on the provided connection.

To run multiple servers, you would setup some listeners (probably TcpListener), pipe their incoming() connections into the Http instance, and then have the Core just run for as long as you want.

Example-ish:

fn spawn_server(http: Http, listener: TcpListener, handle: Handle, factory: YourNewServiceType) {
    handle.spawn(listener.incoming().for_each(move |(socket, addr)| {
        http.bind_connection(&handle, socket, addr, factory.new_service()?);
        Ok(())
    }))
}

fn main() {
    let core = make_core();
    let listeners = make_listeners();
    for listener in listeners {
        spawn_server(Http::new(), listener, core.handle().clone(), || Ok(HelloWorld));
    }
    core.run(future::empty::<(), ()>()).unwrap();
}

[mwanner]

I see, I definitely missed this comment here:

/// This server is intended as a convenience for ...

I guess the name Server fooled me, but I see how it can be useful for "easy" cases.

Together with your example, I managed to put together something pretty minimal that compiles. I even filed a PR, but please feel free to reject, though.

[seanmonstar]

The main pain point here is when you want to use hyper's server, but you need the Core beforehand, or wish to control the Core yourself.

Instead of being given a reference to a Core, it seems a solution could be to allow an Http to "spawn" a server-like task into a Handle. This could end up looking like:

let mut core = Core::new()?;
Http::new().spawn(addr, new_service, core.handle());

// other Core/Handle things whatever
core.run(empty)?;

An alternative API to do the same thing could be:

let mut core = Core::new()?;
let server_future = Http::new().spawn(addr, new_service);
core.handle().spawn(server_future);

This would ease having a server and a client on the same core:

let mut core = Core::new()?;
let handle = core.handle();

let client = Client::new(&handle);
handle.spawn(Http::new().spawn(addr, move || {
    Ok(Proxy { client: client.clone() })
});
core.run(empty)?;

---

This doesn't give the ability to run the Core using hyper's run_until logic, but the run_until seems generic enough that it should probably go in tokio-proto.

[nielsle]

The alternative API looks nice. My ideal solution would require changes to tokio-proto as well. I wonder if the following could be massaged to pass the borrow checker.

core.handle()
     .spawn2(|handle| Http::spawn_new(addr, new_service(handle))

[alexcrichton]

@seanmonstar to be clear though, Server supports a custom Core today already, right? That was the intended purpose of the bind_connection method. So in that sense, just to confirm, you're thinking that this issue is targeted at "make using a custom Core easier", right?

If that's all true then what you proposed seems reasonable. The downside is that it may be difficult to shut down such a server. I'd probably recommend Http::spawn as a method which spawns directly into the reactor vs giving you a future that you then spawn. (although there may be downsides with that)

[seanmonstar]

@alexcrichton Server doesn't support a custom Core, it creates its own. Http can be used with an Handle, though.

Playing with this, it does seem like it doesn't help a whole lot.

Before:

let mut core = Core::new()?;
let handle = core.handle();
let addr = "127.0.0.1:3000".parse().unwrap();
let http = Http::new();
let listener = TcpListener::bind(&addr, &handle)?;
handle.spawn(listening.incoming().for_each(move |(sock, addr)| {
    http.bind_connection(&handle, sock, addr, Echo);
    Ok(())
}).map_err(|_| ()));
core.run(empty)?;

After:

let mut core = Core::new()?;
let handle = core.handle();
let addr = "127.0.0.1:3000".parse().unwrap();
let http = Http::new().spawn(&handle, &addr, || Ok(Echo))?;
handle.spawn(http);
core.run(empty)?;

The benefit of it being a Future is that you can wrap it in another, allowing you to adjust how to shutdown the http future task.