Add hooks sufficient to build task-local data

[aturon]

As currently proposed, futures-core 0.3 will not build in task-local data. The idea is to instead address needs here through external libraries via scoped thread-local storage.

For any robust task-local data, however, we want the ability to hook into the task spawning process, so that data inheritance schemes can be set up.

This issue tracks the design of such hooks.

[tikue]

If #56 proceeds and spawning is removed from task::Context, I expect the value of having a good story around task-local data to increase. In general, I don't really want my executor-generic framework code to stash a Box<Executor> in all the places that need spawning.

Things task-local data would be helpful for:

0. Default spawn (Consider removing spawning from futures::task::Context #56) with task-local data propagation.
1. Storing trace info (Tokio Trace: A scoped, structured, logging system  tokio-rs/tokio#561)
2. Storing other request-local data like user credentials

[Ralith]

I don't really want to stash a Box<Executor> in all the places that need spawning.

I think most places that need spawning are application code, which can just reference an appropriate executor via globals or handles or whatever it likes directly, as done today in futures 0.1.

[tikue]

Yes, that's certainly true. I've updated my comment to clarify I'm talking about executor-generic framework code.

[tikue]

Prior related discussion: rust-lang/futures-rs#937

[Ekleog]

For task-local storage, what is needed is to run code before each top-level future poll (and potentially, if using scoped-tls, code after too).

One solution is to add a hook that allows wrapping every future that comes in the executor into some wrapper future type: this way the wrapper future would be able to execute code around the wrapped future.

Actually, this could even be done by consuming the executor and returning a wrapping executor that first wraps the future with the task-local-wrapper, and then forwards the wrapped future to the wrapped executor.

However, I'm not sure this “consuming the executor and returning it” would actually work: what I actually want is tokio::spawn to use the wrapping executor -- which, AFAIU, wouldn't happen with the consume-and-wrap design.

[tikue]

I messed around with @mitsuhiko's execution-context crate yesterday. It's futures-agnostic and works via TLS, and I made it work with futures exactly as @Ekleog said: a wrapper future:

/// Returns a future that executes within the scope of the current [ExecutionContext].
pub fn context_propagating<F: Future>(future: F) -> impl Future<Output = F::Output> {
    ContextFuture {
        future,
        context: ExecutionContext::capture(),
    }
}

/// A future that executes within a specific [ExecutionContext].
struct ContextFuture<F> {
    future: F,
    context: ExecutionContext,
}

impl<F> Future for ContextFuture<F>
where
    F: Future,
{
    type Output = F::Output;

    fn poll(self: PinMut<Self>, cx: &mut task::Context) -> Poll<F::Output> {
        let me = unsafe { PinMut::get_mut_unchecked(self) };
        let future = unsafe { PinMut::new_unchecked(&mut me.future) };
        me.context.run(|| future.poll(cx))
    }
}

This works quite well! Like @Ekleog, I definitely see the value in hooking into the spawn mechanism, because wrapping every future in context_propagating before spawning is error-prone. Empirically, I've seen context propagation issues in production servers more times than I can count. Such a hook would have to be opt-in, because not every application needs or can use task-local data via TLS.

[Ralith]

A portable mechanism for wrapping all top-level poll calls might have broader applications as well: for example, task-level profiling, or just logging warnings when a poll call blocks for unreasonably long, would call for something similar.

[Ekleog]

I guess the first design decision is: do we want to add hooks to modify top-level poll calls into an existing Executor, or do we want to wrap Executors into other, different-top-level-poll-call-behaviour Executors?

I lean for the first option, because it'd work nicely with eg. tokio::spawn, while wrapping Executors into other Executors would mean the top-level executor would need to be changed.

[aturon]

@tikue @Ekleog A question regarding these executor-level hooks: it seems like if you ever use a library that creates its own executor internally, you would have no way to instrument it with hooks. And of course you could forget to do so on your own executor. Both of which would lead to propagation failures.

Or were you thinking of providing a global hook mechanism of some kind?

[tikue]

@aturon I hadn't considered the multi-executor scenario at all. Do you have any thoughts on a global hook mechanism?

[Ekleog]

Actually, the more I think about this, would it be for task_local or for handling time (timeout shouldn't require forcing the executor, it's generic enough), the more I think that futures should look like this (written in the github comment box, please forgive shallowness of reflexion):

trait Executor {
    type Context: BasicContext;
}
trait BasicContext {
    type Waker: BasicWaker;
    fn get_waker(&mut Self) -> Self::Waker;
}
trait BasicWaker {
    // ...
}

trait Future<Exec: Executor> {
    type Output;
    fn poll(self: Pin<&mut Self>, cx: &mut <Exec as Executor>::Context) -> Poll<Self::Output>;
}

This could be expanded with:

trait TaskLocalExecutor: Executor
    where <Self as Executor>::Context: TaskLocalContext,
{ }
trait TaskLocalContext {
    fn get_task_local(ctx: &mut <Self as Executor>::Context) -> &mut TaskLocalMap;
}
// some magic using TaskLocalMap to make it available as a task-local through some macro

And

trait TimeoutExecutor: Executor
    where <Self as Executor>::Context: TimeoutContext,
{
    type TimeoutFuture: Future<Output = ()>;
    fn timeout_future(ctx: &mut <Self as Executor>::Context, d: Duration) -> TimeoutFuture;
}
// Some macro to make it easily usable from async/await

and [etc.] (actually, even Waker could be moved to specific executors, not all futures require that the executor is able to wake them and some executors may not support waking but just run each future in a round-robin fashion… though that'd maybe be over-doing it)

A Future would then be written like:

impl<E: Executor + TimeoutExecutor> Future for MyFuture {
    type Output = /* ... */;
    fn poll(self: Pin<&mut Self>, cx: &mut <Exec as Executor>::Context) -> Poll<Self::Output> {
        // ...
    }
}

And a future combinator function would look like:

fn run_after_10s<E: Executor + TimeoutExecutor, F: Future<E>>(f: F) -> impl Future<E> {
    E::timeout_future(Duration::from_millis(10000)).and_then(|_| f)
}

Assuming that async/await is able to infer the bounds to set to its Executor depending on its contents, this sounds like a minor inconvenience when writing futures manually (a bit more boilerplate) for much more flexibility and portability (task locals, timeouts, and likely other things that will be implemented by lots of executors and that we've not yet thought about)

I guess this has already been discussed somewhere… could someone point me to where, so I understand the discussion around it?

[Ekleog]

Oh, forgot to mention: it would also allow experimenting with cross-executor task_locals / timeouts outside of std, given that std would only need to provide the Executor trait, and libraries could define TaskLocalExecutor, TimeoutExecutor, etc.

[Nemo157]

@Ekleog rust-lang/futures-rs#1196 has some similar prior discussion. The place where I've always thought this would become far too tedious is in propagating the bounds everywhere it's needed (i.e. when you have a function generic over F: Future you now need to have E: Executor, F: Future<E> and f: impl Future<Output = Foo> becomes f: impl Future<impl Executor, Output = Foo>). Maybe that just won't be a common enough issue to really matter.

[Ekleog]

@Nemo157 Thank you for the pointer! I'll continue discussion there :)

[Ralith]

It's worth noting that tokio constructs its executor(s) lazily, so there is adequate opportunity to hook into them during startup without forcing ugly global state into standard APIs.