Graceful shutdown

[jhpratt]

This PR implements a mandatory graceful shutdown on a Rocket instance, which can be called at runtime.

Calling .get_shutdown_handle() before launch will return a ShutdownHandle. As shutting things down before starting isn't too useful most of the time, there is also rocket::shutdown::ShutdownHandle, which implements FromRequest, so it can be used as a request guard. In either situation, calling .shutdown() will signal the server to shut down.

Per a preexisting comment in the code, I've also converted rocket.launch() to return Result<(), LaunchError>.

Resolves #180.

[jhpratt]

Just a note, it won't be too difficult to add signal handling (ctrl + C), as the signal-hook crate provides for simple abstractions. There's also tokio-signal, which is similar. That can be done in this PR if wanted.

[jebrosen]

currently able to be called using the rocket.shutdown() method

I think that's currently the biggest flaw with this PR -- nothing can currently call rocket.shutdown() once the server is started except for Rocket's own request/response plumbing, which definitely shouldn't be calling it. The sender needs to either be threaded through Request or put in managed state in order to be usable.

As a proof of concept, I think we need a ShutdownHandle type that implements FromRequest. That way, someone could write a #[post("/shutdown")] route or similar. I think we will also need a way to get a ShutdownHandle for a not-yet-launched rocket instance, which can be sent to another thread.

Just a note, it won't be too difficult to add signal handling

Agreed. Let's look at what it takes to add it once we know it can be made to work in a route or fairing.

[jhpratt]

Is there anything currently using managed stated within Rocket itself? If not, wouldn't it be best to preserve that expectation?

Having a ShutdownHandle is definitely feasible, I'd actually just need to revert one of the commits where I included the changes. Off the top of my head, an mpsc sender is cloneable, so it should be relatively easy to implement FromRequest. We could then expose ShutdownHandle (the exact path TBD), and use it like any other guard from the client's perspective.

With regard to handling the sender/receiver errors, should the channel be left open after receiving a shutdown signal? I see no reason to, as it would provide a clear indication that the method is being called more than once.

[jebrosen]

Let's use this error type for launch() for the time being:

enum Error {
    Launch(LaunchError),
    Run(Box<dyn std::error::Error>),
}

[jhpratt]

Would it be better to use hyper::Error over Box<dyn std::error::Error>, given that we know that's the type it will be? After taking a quick look at things, hyper::Error is actually already public via LaunchErrorKind, so it wouldn't be newly exposed.

.spawn_on() could also return hyper::Error over the boxed, since you had wanted to return a Result<impl Future<Result<(), ??>, LaunchError>, per previous discussion. The advantage of that signature is not having to .await to determine if launch failed.

[jebrosen]

Interesting, I really thought hyper was "erased" from the error type. In that case, I don't mind hyper::Error instead of the boxed error after all.

[jhpratt]

Updated to use an enum that can contain a LaunchError or a hyper::Error. I haven't written documentation for it, but it would be similar to LaunchError's.

[jhpratt]

For anyone that might be following this PR, I'm going to attempt adding signal handling once tokio-rs/tokio#1498 is figured out. I believe it should just amount to a couple lines added.

[jebrosen]

This does increase our reliance on tokio; I think we will want to make Ctrl-C handling optional somehow. The overall architecture of this looks fine, but I haven't actually run the code yet myself.

[jebrosen]

Some doc nits, and then I think I'm pretty much happy with this once I test it.

[jebrosen]

Merged as 49709a6, with some error handling followup in a3719c4. Thanks for all the hard work on this!