Consider removing spawning from futures::task::Context

[Ralith]

The Context struct currently conflates two independent concerns: allowing futures to arrange to be notified via Wakers, and allowing new tasks to be spawned. I don't believe these belong together. Wakeup handling is useful to practically all leaf futures that aren't serviced by kernel mechanisms (i.e. that aren't leaf I/O futures), so it makes sense to ensure these facilities are passed down to every leaf. By contrast, very few futures require the ability to spawn tasks, and those that do are typically in application code (for example, the accept loop of a server) where an executor handle can be easily made available. In the rare case where library code genuinely needs to spawn new tasks, this can be easily accomplished by explicitly taking an executor handle, or by returning an impl Stream<impl Future> whose elements can be spawned in whatever manner is appropriate to the application.

The specifics of spawning a task can also vary considerably between executors in ways the generic interface exposed by Context cannot support. For example, applications which require non-Send futures or which can't perform dynamic allocation cannot make use of Context-based spawning at all. This not only leads to awkward vestigal API surface, but also presents a subtle compatibility hazard: code using an executor that does not support spawning via Context will compile fine when combined with libraries that assume one, but fail at runtime when spawning is attempted. By contrast, if the ecosystem standardizes on returning streams of futures, spawning (and guarantees such as Sendability of futures to be spawned) naturally becomes explicit.

cc @carllerche, @Nemo157

[carllerche]

I am also strongly against including an executor with the Task context.

The argument for including it is that it always allows spawning in all future related context. However, this is not true. Instead, adding a context spawner adds yet another way to spawn, adding to the confusion.

In my experience, I have rarely needed to be able to spawn from library code. The only case in which it comes up is very high level APIs (like hyper's high level API). All other libraries never spawn. Instead, they return objects that allow the caller to control how they get spawned. For example h2 does not require spawning, instead it returns a Connection object that the user is expected to drive (usually by spawning).

In all of my cases where spawning from a library is required, the spawner passed to the context object is insufficient. This is because it is a) a trait object and b) forces Send on spawned tasks, and c) would limit the flexibility of the library.

a) and b) are related. Being a trait object prevents using typed executors (executors that are designed to only execute tasks of a specific type, not T: Future<...>). Also, requiring Send forces the library to only work in Send contexts even if the library is built to handle both Send and !Send.

Instead, tower-h2 takes an E: Executor<Background<T>> where Background is the task that needs to be spawned and T represents a user provided action. In this case, if T: !Send, then the executor provided must be a "current thread" executor. By representing the executor this way, tower-h2 does not force Send.

c) If the library is built to only be able to spawn with a context object, this prevents spawning from drop fns. Being able to spawn from a drop fn is necessary to be able to run async cleanup code given that drop cannot block.

Because of these limitations, I do not believe the spawn argument to context will be used much at all with Tokio, instead Tokio's executor system will be the preferred method. I do not believe it to be a good idea to include a spawn argument in the type provided by std when it is not globally useful.

[MajorBreakfast]

@carllerche Thanks for this feedback. I don't have an opinion on this yet, but I find it immensely important that we discuss this properly. Here are some of related thoughts:

• Make the context generic: Rust loves to enforce everything through the type system. A generic context would make it possible to make clear what things are needed to run a future, e.g. Future<Spawn + tokio::Runtime, Output = ()>. This would make things very explicit and possibly very optimiser friendly.
• Investigate whether a task_local! macro, like @Ekleog proposed in Build task_local! data from a Future rust-lang/futures-rs#1187 is interesting as a building block

[Ralith]

@MajorBreakfast both of those options seem to presuppose that it's worth passing an executor handle implicitly to every single future, which I don't think should be assumed, particularly given the rarity of spawning in practice.

[MajorBreakfast]

@Ralith Focus not so much on spawning, but on the event loop example I gave. For instance Tokio's futures require that there's an event loop around. Without a generic context the way you find out if there's an event loop around is by seeing it fail at runtime. With a generic context, there would be a trait bound that enforces that the context implements the event loop functionality. Same story for spawning. Spawn would be just one of many traits the generic context could implement.

[carllerche]

IMO that would be a good route to explore regardless of spawning. The needs of Tokio are very different than embedded, etc...

[Thomasdezeeuw]

I'm also in favour of removing spawn from the future's context. But that only leaves waking in the context, which, to mee atleast, is really the only required part of the context to run (poll) the future. Maybe rather then using a generic context, futures should only require a Waker as argument. To me that is only thing that is essentiel to (effectively) run/poll a future. All other functionally can be provided by the runtime system type.

Another argument against a generic context is actually the lack of being generic. Futures have proofed to be a difficult subject to learn and understand. Adding more complexity, via a generic context, might result in people just saying "just use Tokio's context/types" (or any other runtime). This would have the adverse effect on the genericity (is that even a word?) of Future types build by the ecosystem.

[aturon]

I agree with @Thomasdezeeuw.

I think we should remove the concept of Context entirely, and simply pass poll a LocalWaker, making wakeups the only fundamental requirement that applies to all futures. While I had initially hoped that we could have a useful notion of "default executor", it seems too fraught, and as many people have pointed out, it's somewhat rare to require spawning in library code anyway.

[cramertj]

+1-- the huge advantage here for me is that we get to remove Context and just pass &LocalWaker into futures. IMO this drastically simplifies the API and makes things easier to understand.

Also, @carllerche made the excellent point to me on Discord that libraries which return an impl Future type for the user to run, though less ergonomic, allow for a great deal more flexibility in terms of how the user schedules the object (including allowing non-'static lifetimes, as well as choosing whether to make the type Send or not).

Combined with the fact that end applications will usually make use of a thread-local spawner anyways for convenience (since they're available even outside of async contexts) I think Context isn't pulling its weight. I'll work on drafting a PR to remove it so we can push ahead, but please speak up if you think Context should stay! (There will, of course, be time to re-review this decision when the final std::future RFC comes up for review, but ideally we'd have consensus on this and land the desired API in std before that.)

[Ekleog]

FWIW, I use Context to spawn a future in https://github.com/Ekleog/erlust/blob/master/erlust/src/spawn.rs#L14, to mirror tokio::spawn. Removing the Context would mean I would have to either:

• just return the Future and let the user spawn it themselves (which isn't as user-friendly IMO, especially given this future wrapper must be run directly as a task -- and in particular nesting two such future wrappers would most likely wreak havoc), or
• depend on tokio to be able to use tokio::spawn

Both of which options sound… unhelpful.

All that to say, I think the ability to spawn from a future does make sense, and a blanket removal would be a step backwards. Then, @MajorBreakfast's solution of having some contexts implement the SpawningContext trait and some not implementing it would solve the issue of having contexts unable to spawn, while leaving open the opportunity for me to require a SpawningContext.

[Ralith]

I'm not sure what that code is for (things that want to mirror tokio::spawn are generally executors), but it could perfectly well just take a reference to the spawner you want to wrap explicitly instead of relying on it being bundled into the context.

[Ekleog]

I should have given more context to this code indeed.

So basically, here there are two kinds of Tasks: normal tasks, and actor tasks. Actor tasks have additional setup/teardown, which is done via LocalChannelUpdater. Hence my not wanting to just return a future and let the user decide how to spawn it: the user could then chain other stuff outside of the LocalChannelUpdater or put two LocalChannelUpdaters one inside the other, which would do Bad Things.

However, I don't want/need to write an executor, I can just run the system on any executor that supports spawning tasks. Hence my not willing to just use tokio::spawn, as that'd force the user to use tokio.

The solution of requiring the user to explicitly pass a spawner is a solution indeed, but makes it way less comfortable to the user, as they would have to actually pass said spawner all around their stack, which would infect just about all the code… or just use (an equivalent to) tokio::executor::DefaultExecutor::current(), which would just move things that could be made generic over all executors into just the tokio executor. And given I don't see any reasonable use case for anything else than the default executor… I'd rather just hardcode this and have an easy-to-use API, but without depending on tokio.

BTW, the code I'm referencing here that makes the difference between actor tasks and regular tasks is more or less a manual implementation of the task_local! macro @MajorBreakfast mentioned in #56 (comment)

[Nemo157]

they would have to actually pass said spawner all around their stack, which would infect just about all the code

having some contexts implement the SpawningContext trait and some not implementing it would solve the issue of having contexts unable to spawn, while leaving open the opportunity for me to require a SpawningContext

These result in having to change pretty much the same sets of code, either you need to be generic over a Spawn and pass an instance of that everywhere, or be generic over a SpawningContext and have that implicitly passed. (One major question with the latter is how it would interact with async fn, there's currently no way to add these generics to the returned future. The former is much easier to integrate.)

If you're writing an actor system that has task_local!s for actors, couldn't the spawner just be one of those task locals? You init the actor system with an initial spawner for the executor it's running on, then have that implicitly passed around to all the actors (maybe with a way to contextual change it for sub-systems that want to use an alternative spawner).