Core::run_until_finished

[Ralith]

This provides a considerable improvement in ergonomics when you want to stop executing when all work has been completed.

I'm not super familiar with this level of the stack, so please confirm that Inner::live is correct before merging.

Better ideas for the method name very welcome.

[alexcrichton]

Thanks for the PR! I'm a little hesitant about adding this though because applications always don't have knowledge of all tasks that they're spawning. For example the tokio-signal library spawns a task that intentionally never exits and it's break this usage pattern. In general though you don't know whether the async libraries you're calling are going to be spawning tasks (like how you don't know whether sync fns spawn threads) so waiting for all tasks to exit may not always be right.

Mind elaborating on some of the motivating use cases here?

[Ralith]

Mind elaborating on some of the motivating use cases here?

It's common to want to support a graceful shutdown by returning control from the core after all work has been finished. For example, you might want to shut down a web server on SIGINT, but only after completing all outstanding requests. To accomplish that in current tokio, you need to contrive a future that will complete only in exactly that circumstance, which requires manually tracking outstanding requests in addition to no longer accepting new ones.

Notwithstanding the implementation details of tokio-signal, with the proposed interface no such bookkeeping would be necessary: you would kick off the main connection-accepting task structured to terminate on SIGINT, and then simply run_until_finished to get the desired behavior.

I'm a little hesitant about adding this though because applications always don't have knowledge of all tasks that they're spawning. ...

This is a good point which I hadn't thought of when I drafted this PR. However, I believe the proposed high-level semantics are very useful in principle--we just need a way to distinguish tasks that should and should not sustain execution. It seems to me that most cases where code might spawn a task behind-the-scenes that will run forever, rather than returning a Future or Stream directly, should not cause execution to continue: they typically exist only to drive other futures that are ultimately consumed by the top-level application tasks, so once those have terminated, there is no benefit to further execution. For example, tokio-signal doesn't do anything on its own.

The proposed semantics are basically the same as those implemented by libuv. They allow for the design pattern of tokio-signal by allowing you to explicitly tag a task as "unreferenced", which indicates that it should not cause execution to continue. I've found this pattern to be very convenient.

I believe something analogous to libuv's approach would make sense here. A mechanism to construct "backround" tasks could be introduced, and Inner::live could be modified to only count regular tasks. Does that sound reasonable?

[alexcrichton]

Thanks for the comment and sorry for the delay, but yeah having a mechanism for "helper tasks" sounds good to me, there's a number of libraries I think I'd use that for. I'm not entirely certain what such an API would look like, but with that I think I'd be ok to add this.

[stepancheg]

In general though you don't know whether the async libraries you're calling are going to be spawning tasks

For me that's the reason for this patch, not against.

When gracefully shutting down the server, I want to be sure that all tasks completed, no task left doing unaccounted work.

My use case is this:

I want use single event loop to start server, start another server, stop first server, start third server and so on.

When shutting down the server, I want to keep event loop running, and make sure there's no leak of any resources. To check this property I could write test like this:

let core = Core::new();
let server = Server::new_in_loop(core);
// do something
server.shutdown().wait();
core.run_until_finished();
// at this point I know there are no leaks

like how you don't know whether sync fns spawn threads

(BTW, I think it was an error when std was designed, that threads detach in destructor. IMHO threads should join in destructor for similar reasons.)

[alexcrichton]

I don't disagree there's a use case for this. I disagree we should land this as-is. If we added a way to explicitly spawn a "background task" then we would be able to land this.

[stepancheg]

If we added a way to explicitly spawn a "background task" then we would be able to land this.

Doesn't Handle::spawn spawn a background task?

[Ralith]

No, Handle::spawn is routinely used for things like handling client requests.

[stepancheg]

What's the difference?

[Ralith]

A background task, in the sense used here, is a task that should not sustain the event loop, because it exists only to support the execution of foreground tasks. The distinction is purely a matter of programmer intention, which we need (and do not currently have) an API to express.

[carllerche]

The fundamental issue right now is that a single core can be used by many independent libs and there is no way to signal shutdown.

One option might be to have Handle::terminating() -> impl Future<...> that provides a future indicating that the core wants to shutdown.

[Ralith]

The fundamental issue right now is that a single core can be used by many independent libs and there is no way to signal shutdown.

I think it's usually pretty clear whether a given task is the core of a process or exists only to support others. The approach previously discussed is IMO a complete (and simpler) solution, I just need to get around to drafting a Handle::spawn_background API.

[carllerche]

I think it's usually pretty clear whether a given task is the core of a process or exists only to support others.

Clear to who? The spawner or the core? I mean, in general there needs to be a way for tasks to have the opportunity to shutdown gracefully...

Actor systems use supervision trees + shutdown messages to do this :P that is probably more than we want to do.