Explicit task::Context argument to poll

Learnability, reasoning, debuggability and no_std-compatibility improvements achieved by making task wakeup handles into explicit arguments.
Rendered

The discussion about this in #1 mostly centered on the following sentiment:

If the motivation for introducing Context is solely learnability, isn't there lower-hanging fruit (like docs) that don't have ergonomic downsides?

I personally do agree with this sentiment, so to me the key question is if a Context argument has benefits beyond learnability. The RFC mentions some (related to no_std compat), but in addition, one effect of having this explicit argument is that you can tell, at a call site, whether a given method involves potentially queuing the task. That seems potentially helpful information to have locally, especially when navigating a foreign codebase.
I think it's a really tough call, and I've long basically been on the fence on this one. But it certainly does give me pause that many, many people in the Rust community who have learned futures have continued to ask for this argument to be explicit.

@aturon The majority of the community feedback in the original thread is the opposite. The lack of an explicit argument has not been the primary blocker to learnability. This is also feedback from people who spend time actively teaching futures.
The simple fact is addressing the learnability with docs has not been tried. Changing the API at this point to address learnability is premature given the state of documentation.
I think this comment provides valuable insight into learnability.
Also, the community feedback from those who spend a lot of time implementing Future is that the explicit context argument is just additional boilerplate as in most cases it will not be used directly, instead just ceremoniously passed along.
My personal opinion is that adding an explicit task argument is going to make the experience of working with futures noticeably worse to solve a problem that could be handled with a couple paragraphs of docs + a few examples.
As @djc said:

It seems the main disadvantage for learnability (that is, leakiness of the abstraction) would not be on most authors of Future impls, but only on a pretty select set that implement fairly low-level Future impls (at the level of mio itself, for example). In all of the cases that I tried (dealing with network protocols, near the tokio-io/tokio-proto level of abstraction) it was just composing higher-level futures out of lower-level ones, meaning passing around Context would just be boilerplate.

@carllerche
As I said in my comment, I don't think the learnability argument by itself suffices to motivate this change.

The majority of the community feedback in the original thread is the opposite.

There's been a mix of sentiment, including on the very long-running thread on this issue. All I'm saying is that, as you know well, it's something that people have been asking for for quite a long time, and that gives me some pause.
Also, last time we spoke about this, you were very much on the fence. I'm curious what's changed your opinion?

The summary uses the word "learnability", but then the RFC never uses it again.
In the main paragraphs, it solely uses discoverability, which is clearly different. Discoverability can also be improved by mentioning the existence of the task in the documentation (possibly in the method docs).
The technical arguments for this are very underdeveloped in this RFC, with a mere 2 sentences.
I'd find it more convincing if I found a sketched no_std example there. I find that a worthwhile discussion path, sadly, there's almost no one coming up with a proper example.

@aturon mostly that I thought I was crazy and I was the only one who thought the thread-local strategy was a good idea. So, like I said, I wasn't going to block the decision either way and let it go to RFC.
The community feedback tips me back in the "thread-local" camp, but all I'm doing is putting my thoughts on the record and let what happens happen.

@skade Yep, I agree that this RFC will need to make the non-learnability motivations much stronger to be a solid sell.

Could the expected integration of futures w/ async / await be included here?
For example, would crates like h2 be updated to use async / await instead? Would future implementations be able to use async / await?
How is the task context accessed from async / await fns?
It seems to me that if writing poll fns manually goes away, then the ergonomic downside is significantly reduced, but I have no idea if that would happen or not.

How much smoother will that transition actually be with an explicit task argument?

As someone who has implemented a number of Futures (in crates like Thrussh and Pleingres, and other unreleased projects), I'd actually also like to question the learnability argument.
I'm not saying futures are easy to learn, it did take me a while to figure out how to implement poll correctly for complex types (for instance an SSL wrapper with bounded memory would be difficult to implement, I don't think tokio-openssl does that), but I don't remember any time where thread-local storage has been a problem: most of the time, my errors were in the logic, i.e. I would return Async::NotReady where in fact no IO operation had happened (or more generally, no other future had returned Async::NotReady). For instance, the current API makes it slightly tricky to test whether at least one of several futures is ready, but I can't think of any better way, because the logic is not simple anyway.
I believe the main thing I had to learn was that, and I fail to see how an extra argument to all functions would make it more explicit.
I must admit I still don't really know how futures get registered for wakeup, but my current mental model seems to work well enough, and I don't believe the extra Context argument would teach me deeper things about that.

I feel the whole learnability argument has become somewhat of a red herring (though I do agree with it, as I did not get where wakeups were registered at first and an explicit context would help with that).
Moving on. I'd really like some input from people that want to use futures in no_std environments. If removing TLS makes it possible (or perhaps just makes it easier) to use use futures in such environments that is an indisputable advantage of the explicit solution. Perhaps we may deem those use cases not important enough but that should be carefully weighed against the advantages of TLS.

I think it is good to explore the design space here, however caution is of course always advisable.
I find some precedent in keeping state off the call stack in the regex crate, which if memory serves uses some thread local storage to make regexes thread-safe to use (whether or not this feature is widely used I won't argue here). In this case, the explicit version was considered too unwieldy, though it would presumably have had a tiny performance benefit. Of course, futures should be minimal cost even with fine-grained tasks, so what is acceptable for regex may not be acceptable here.
Perhaps @withoutboats recent design trick for Generators could be applied here, too. Make an explicit version for low-level stuff and build wrapper types that bridge them to the implicit higher level trait?

@Marwes
To be clear, you can use futures-rs from a no_std environment today (see here.
Making the argument explicit would arguably improve the situation for no_std, but AFAIK, nobody has hit a blocker to date.

cc @zoxc @Diggsey

I am a casual reader of the RFC, however I have a question which other casual readers might have as well :)
The RFC shows changes in the poll signature, so it's clear how the implementation of the Future changes.
But what happens to the (majority) of the code that just uses futures? Should I thread this Context through each combinator? Probably not, but something has to change on the usage site?

@matklad If you're just using combinators, nothing should change.

@cramertj hm, I am still puzzled by

but in addition, one effect of having this explicit argument is that you can tell, at a call site, whether a given method involves potentially queuing the task.

I think it means that something changes for the users of futures? It probably would be helpful to see a code example along the lines of "here an echo service in async hyper before and after". Or is there literally no change unless you impl Future for MyType?

@matklad There should be literally no change. The sentence you quoted refers to functions like poll_read and need_read which are used by implementors of Future types.

Just thinking out loud without quite knowing where to put it:
I am wondering how much confusion around poll comes from the simple fact that it comes first in the Futures docs and doesn't make quite clear it shouldn't be called from user code in it's docs. It can be deduced from the text when you read it in full, but it never quite spells it out.
Poll is only of interest when implementing the trait and for executors.

it shouldn't be called from user code

I guess this depends on what you consider "user" code. Everyone I know working with futures currently has to implement futures themselves for one reason or another.

I should clarify that: outside of an executor context. poll should either be called from poll or from an executor, not in the code constructing the futures.
I can see an explicit task clarifying that situation.  */)
}

Then again, I wonder how much of an issue that is. People might or might not hit that, reach for an example and go on from there.

I've written a fair amount of futures code over the last two years. I didn't find the current model particularly difficult to learn, though I already had a large amount of experience with event loop and multithreaded programming from my days working on Firefox.
I think making the Task argument to poll explicit is probably a good idea. I'm slightly less convinced that its worth the disruption to the futures ecosystem at this point, though I expect that's likely to be manageable. In the past I've found myself writing a lot of futures chains that boil down to something like "if we have the data already return it, otherwise do a computation asynchronously and return it when done".  If you attempt to poll that chain outside of a task, the "have the data already" case generally doesn't needs to wait and will complete just fine. But when we fall into the asynchronous computation branch we'll panic since we cannot wait without a task.
Someone above said that the hardest problems to debug are usually those where a future's poll impl fails to poll another future or ensure that the task is waiting for something to wake up. I tend to agree with that. It would be nice to check that statically but that seems difficult at best (since a future could poll 10 child futures and as long as any one of them waits or polls something that does it's ok).
I am curious what the intended use case for task-local storage is. I've never found myself wanting that. Typically I either have an object that I want to share across futures but it cannot be used concurrently, in which case it goes into TLS, or I have an object that I don't want to share across futures. Perhaps there are other execution models where the task is more meaningful but in futures-cpupool or tokio the task you're running on seems largely arbitrary.

I am curious what the intended use case for task-local storage is.

I am currently using it to build a tracing system. The request context can be stored in a task-local and libs can use that when adding tracing data.
This is similar to how tracing is done in finagle.

One thing I've had issues with is knowing when to use the wake API.
Just exposing the context is not as helpful as using the type system to help educate me. That said, exposing the context is the only way to better utilize the type system.
@cramertj Would it be possible to not just expose context, but use the type system to help educate users about when they need to use the task apis (similar to how, must_use on Result educates users about error handling)?

I think making the Task argument to poll explicit is probably a good idea. I'm slightly less convinced that its worth the disruption to the futures ecosystem at this point

I want to echo @khuey's concern -- I think that if we were starting over again, it may be reasonable to have a Context argument, even at the cost of the added boilerplate for most Future impls.
My larger concern is that this is an extremely disruptive API change.  Every single existing impl Future will need to be updated. Are the benefits really worth this cost? It seems to me that there are far less disruptive changes that could be made to address the motivations otherwise.

I'm generally in the camp of "explicit is better", but I feel like I don't know the whole story here -- what was the original reason for hiding the Task argument in TLS?

I don't have much experience with futures yet, but I do plan to integrate it into embedded and OS projects. As I understand it, it is possible to use futures in a no_std context, but it requires the use of the unsafe task::init function. This function makes it possible to define an own thread local storage mechanism, but it operates on raw byte pointers and seems very hacky overall. The proposed explicit argument would make that use case considerably safer and cleaner.
So for me the question is: What is the big advantage of thread local storage here? The only argument I found was that it reduces boilerplate when implementing Future, which in my opinion does not suffice to motivate the flexibility loss.

@olix0r Disruption is certainly an issue, but if #1 goes through, all futures code will need to be touched anyways, and this change is rather straight-forward.

@olix0r
s/fn poll(&mut self)/fn poll(&mut self, context: &mut task::Context)/g + s/,/poll()/.poll(context) would take care of 80% of all changes necessary for this PR so that shouldn't be a large determining factor.

I'm not a big futures user, but ('and therefore', in one case) I want to weight in on a few points:
learnability: you should do both - improve the API (if we conclude that it is an improvement) and improve the docs. I see no reason why docs should block a better API, if anything, I would want the best API first and then do docs - IMO docs are necessary when something is not obvious from the API.
thread-local vs explicit: I've been hitting this discussion in a number of places and futures might be special in some way, but in general I have always ended up in favour of an explicit argument: TLS has many of the downsides of global variables - you can't clearly see inputs to a function, you can't add a second context to a thread, you can't reuse code as well, no encapsulation, non-locality, harder to test, etc, etc. Sure TLS doesn't have the concurrency issues of globals, but they are just as bad in all the design/engineering aspects.
Disruption: while futures/async io are awesome work and for sure will be hugely significant, I think it is worth getting some perspective. Although there are some users making heavy use of the futures ecosystem, for most Rust users futures are something really cool for the future. Looking at the Rust community as a whole, even major changes to futures will not be very disruptive in the grand scheme of things. Put another way, the future community of futures users is so much larger than the current community of futures users that significant disruption for a marginally better future is worthwhile.

I kind of feel like this has been discussed to death on the previous issue 😒

I feel different there, this thread has raised quite some detailed issue the previous one was lacking.

learnability: you should do both - improve the API (if we conclude that it is an improvement) and improve the docs. I see no reason why docs should block a better API, if anything, I would want the best API first and then do docs - IMO docs are necessary when something is not obvious from the API.

I don't agree with the sentiment of good API over docs. The futures crate itself is full with good API that has no proper context (heh!) and is therefore hard to learn. The importance/non-importance of certain functions is very hard to get.

To me Rust is a language that doesn't hide anything.
TLS feels like a hard coded assumption. Conceptually I don't want any mystery ingredients when cooking up a solution.
That being said. I haven't had the need to write a custom future in the past months, so practically this will  not affect me much.
As far as naming goes. Context seems alright, but just to throw some stuff out there:

PollRegistry
Planner
Environment
Stager

Put another way, the future community of futures users is so much larger than the current community of futures users that significant disruption for a marginally better future is worthwhile.

And I'd bet that a significant part of current users know they are using a 0.1 crate, and expect breakages and transitions, so this doesn't feel like a problem.

TLS has many of the downsides of global variables

Now that is a much much stronger argument than learnability.

Can somebody explain how futures::task::init would be used? I would expect that, for example, when using coroutines in #[no_std] environments without thread_local I would just need to make the Task static instead.
futures::task::init seems like a way to do that, but I don't see how one would know how much static storage to allocate since the get/set functions deal with pointers but don't have an associated size.
What am I missing?

This is great, using TLS for this is a clear design mistake as argued by the previous commenters.
I'd also suggest naming the argument "notifier", "wakeup" or "callback" and NOT "task" or "context" since what is being passed is just the callback and whether that's part of some sort of "task" or "context" is an internal implementation detail of whatever is calling poll().
Also, this change can be done in two steps, where after the first step poll() has an explicit wakup argument, but poll() implementations that use Tokio Read/Write streams still use a TLS variable to pass the notifier to the streams, and in the second step an alternative mechanism is devised (some options were discussed in futures-rs#129 ).

thread-local vs explicit: I've been hitting this discussion in a number of places and futures might be special in some way, but in general I have always ended up in favour of an explicit argument: TLS has many of the downsides of global variables - you can't clearly see inputs to a function, you can't add a second context to a thread, you can't reuse code as well, no encapsulation, non-locality, harder to test, etc, etc. Sure TLS doesn't have the concurrency issues of globals, but they are just as bad in all the design/engineering aspects.

As someone who's working on legacy codebase parallelization projects where people are very eager to think "Oh, I know, I'll just slap a thread-local on this!", I would be even harsher. Thread-locals are, from my point of view at least, even worse than global variables, because they cumulate almost all of the drawbacks of globals with some new ones of their own.
They are worse, because they come with a lot more black magic. The rules about where and when you are allowed to create a thread-local are more complicated than those describing where you are allowed to create a global/static variable. There is a hard OS limit on the amount of thread-locals that you are allowed to have, which is system-dependent and cannot be known in advance. Some OSs don't support thread-local storage at all. And on those which do, the performance characteristics of accessing thread-locals can vary by orders of magnitude depending on where data accesses come from, leading to unexpected performance disasters whenever codebases are refactored. Finally, and this is especially true in library code, initialization and finalization of thread-local data are more tricky than those of other data, although in Rust the initialization part is mitigated by taking a hard stance against life before main.
In addition to all the aforementioned drawbacks, thread-locals are also worse because they provide a false sense of security. While the harmful interactions between global state and multi-threading are very well known, due to the mere fact that thread-locals are data-race free, people start to think that suddenly using implicit state is all okay again, and get away with it for a while. Until someday, inevitably, they end up migrating work across threads (be it as part of some sort of data reduction process or because they are using some kind of migratable coroutine library) and suddenly their code breaks in new funky ways. It is generally harder to observe this kind of bugs than it is to observe race conditions on global variables, because they only manifest on work migration, which in good code is an infrequent event, and is generally quite detached from the logic that actually uses the thread-local storage.

Never used futures in Rust, but looking at poll signature I definitely did not know that a thread local is required to call poll correctly. I am more in favor of explicit here.

As a futures beginner, I think that tasks are very confusing as they stand.  Leaving tasks in thread_local means polluting global, which I feel is a bit of a code smell.
Having this is an explicit function argument is a great way to reduce the opaqueness of the futures ecosystem, considering that if you don't manage tasks you can get panics quite easily.
Here's the thing:  Whether this is an explicit argument or not, you are dealing with this anyway, just in a more obscure way.
As a though experiment: Try thinking about creating a new executor from scratch, just looking at the API contract.  What would be easier?
As a side note:  I'd love to do away with this notify stuff completely: it is too tightly coupled to mio and evented, etc...  I'd like to see futures used for things that are more general (look at Streams in Java 8 or even RX and friends).  Although I don't know if that's possible.

I don't know how much this is relevant, but thread local storage has interesting problems for interaction with other parlization frameworks:

in May it results in undefined behavior
in rayon  it uses thread local data from with the rayon thread pool (not the current thread pool.

There may be other examples. IMO this is an argument in favor of this RFC since being able to combine parallism frameworks in the rust ecosystem is a really good thing (but my knowedge is very limited).

This seems to be a good thing. Dependency injection is always kind of a bit ugly, but in lieu of Rust having a systematic effect/handler system that could allow you to mark the fact that external effects are being required, and swap out the handlers of those effects (like OCaml's multicore does), this seems like the better approach. It would be useful to see some examples of code that would be made worse in light of this proposal though, to make the objections more concrete.

significant disruption for a marginally better future is worthwhile.

If it were measurably better, then I'd be for it, even though it will hurt me. However, I'm not sure the "marginally better" here actually solves the bugs people run into, while it also hurts all current code.
Of all the stumped users I've interacted with, by far the most common bug made when implementing Future is not that they didn't know there was a thread local task to notify. It was always that they had grouped together a bunch of child futures, or tried to chain a few, and didn't know that the specific combination they had polled never needed to notify the task.
We shouldn't fool ourselves in thinking that by adding an argument, those bugs are going away. But what we are doing is saying "thanks for exploring, too bad the new shed color broke all your code".

I don't mean to sound dismissive of other people's suggestions, but I do think some care should be taken when actually reading the comments on this RFC: many who have very little or no experience writing futures want the change, and many who have written a lot of futures don't think a change is needed.

I think the explicit handle will help intermediate users who want to understand the details that make futures work, but for beginners learning futures the learnability argument is much less clear. It's another element that must be at least briefly explained early on in a tutorial and increases the risk of the learner feeling overwhelmed by the syntax. From a didactic point of view it would be better if the detail of how futures are scheduled did not get in the way early on and was left to more advanced documentation.

Everyone I know who works with Rust is holding off writing large, futures-based codebases because it isn't considered stable. If we can't introduce a breaking change in 0.1 we have failed to set expectations. The message we should be sending the ecosystem is: Feel free to experiment, but it isn't done yet. Maybe there's a systematic approach to make this better in the future (feature flags for crates?).

"thanks for exploring, too bad the new shed color broke all your code"

What I'm lacking are arguments for imposing a special TLS-based contract which are more compelling than having an argument providing the task context. I don't agree that choosing between the two is a matter of aesthetics. The burden of proof should lie with doing something special, not in doing something ordinary.

Everyone I know who works with Rust is holding off writing large, futures-based codebases because it isn't considered stable.

As someone who does work on a large futures-based codebase I think this breaking change is ok. Fixing things up is largely mechanical. We'll be fine.

What I'm lacking are arguments for imposing a special TLS-based contract which are more compelling than having an argument providing the task context.

@udoprog Such arguments have been provided at length over at this issue. For a concrete example look at how simple and elegant it is for flate2 to abstract over blocking and non-blocking readers and writers. It seems to be a big win for implementation reuse, if the handle had to be plumbed down everywhere it would mean a lot of reimplementation work for libraries to have interop with async and tokio.

I'll chime in as another person who's spent a lot of time working with futures, and is strongly in favor of this change. The use of TLS makes the mechanics of futures extremely mysterious. Sure, it's not the single sole cause of bugs, or even the absolute most common one, but it does make things worse. If the mechanics of the system at large are more immediately apparent, then it becomes clearer why a certain pattern of polling child futures is needed, which makes it easier to learn and consistently implement.
I also strongly agree with the general principle that rejecting concrete improvements to a 0.1 prototype crate due to minor inconvenience to its existing users defeats the purpose of prototyping.

I'm lacking are arguments for imposing a special TLS-based contract which are more compelling than having an argument providing the task context.

An example that @leodasvacas linked, it greatly helps adding in support for IO things that can work over non-blocking IO automatically.

rejecting concrete improvements to a 0.1 prototype crate due to minor inconvenience to its existing users defeats the purpose of prototyping.

Just to clarify my previous comment, I'm not saying we can't change. I was saying a gigantic break without measurable improvement feels bad. I said I'd be in favor of the change if it did definitely fix things. I haven't seen an argument so far that shows it to be much better.

I'm very much in favor of this. I found the implicit thread-local task to be very confusing. In order to learn futures, I wound up extending the API with a context, just so that I was clear on how things actually worked:
https://www.reddit.com/r/rust/comments/76t442/simple_easy_to_understand_futures_a_more_explicit/
Once I did that, things clicked for me.
I also found that future combinators get messy, especially as the types get complex, and sometimes custom future implementations are a good way to control that complexity. I think that custom future implementations will be relatively common, and not "experts-only" territory, so I am in favor of this change as it makes it clearer how futures work under the hood, and how to implement them.
I do not think that the argument for compatibility with blocking IO is a strong one. I think that async IO and sync IO are different, and I would like them to look different. I also think that it sounds like a source of future headaches if if blocking IO implementations don't do the right thing with respect to async, and cause programs to mysteriously hang.

Note re: using libraries (like openssl) that work with Read/Write only: it's still possible to use them after this RFC. We can provide an explicit wrapper that transforms from e.g. AsyncRead to Read, using TLS internally. The wrapper would provide a scoped means of installing the current task into TLS, which you'd need to use surrounding the call into openssl.
In other words, we can basically sequester today's use of TLS into a small, explicit wrapper that's used in the few cases where we want to leverage an existing Read-based API.
It's hacky, but not really more so than today's setup. But it isolates the hackiness to the AsyncRead/Read barrier.
So, IMO, interfacing with Read/Write-based APIs is not a big factor one way or the other.

I'm reminded of https://golang.org/pkg/context/ . It's not the same thing, but somewhat similar. We may want to either draw explicit parallels or name it different so as not to confuse people.

As someone who has written a nontrival number of Stream and Future impls, I support making the context explicit.
A year ago I tried porting futures-rs over to this style, and the results seemed promising: https://github.com/rust-lang-nursery/futures-rs/pull/374.
I worry that by tying Futures to thread-local storage (or to the unsafe init() function in the no_std variant) we would be unnecessarily constraining how and where Futures can be used. My concern is similar to what @vitiral was getting at above. As Rust evolves and users discover new patterns of programming with it, I expect that people are going to want to use futures-rs in new ways that are unimaginable to us now. Like maybe -- and this is just a wild stab at what such things might look like -- someone would want some kind of framework that intercepts or wraps the context as it gets passed along a chain of Futures.
The best we can do to prepare for such demands is to be humble and to keep our interfaces be honest.

With async/await using the generator syntax, the context argument would be hidden in the generated code. How frequently do we expect users to write explicit futures or interact with the task context when async/await is implemented?

I imagine that the standard way to pass context in Rust is using parameters. C api's like rand()/srand() and GetLastError()/errno seem to be less popular because global state turns out to be a bad idea.
So, the default here would seem to be using a parameter. Given that, I don't see sufficient justification for using TLS. The only reasons to not do it at this point are that it is a breaking change and that it would make transparent (whether correct or incorrect) usage of Read/Write harder. But given the size of the commit to change flate2 to support tokio, I don't really take that benefit seriously.
I don't really see why TLS is being considered other than it being a decision that was made initially and it's easier to continue with the current implementation than it is to change it.

Everyone I know who works with Rust is holding off writing large, futures-based codebases because it isn't considered stable.

This RFC is not proposing to introduce breaking changes where you think. Implementing Future is mostly required when implementing new protocols. "Large future-based codebases"  will likely use combinators instead, and these won't change.
@Ralith:

If the mechanics of the system at large are more immediately apparent, then it becomes clearer why a certain pattern of polling child futures is needed, which makes it easier to learn and consistently implement.

In all cases I've written of this, I had to think whether each line was doing IO. An extra parameter might make this more explicit, but that's something you have to think about anyway.
This RFC might make it more general than just IO, which is in favor of this RFC.

I don't mean to sound dismissive of other people's suggestions, but I do think some care should be taken when actually reading the comments on this RFC: many who have very little or no experience writing futures want the change, and many who have written a lot of futures don't think a change is needed.

Maybe we should measure this more precisely. This RFC would have helped me maybe a little at most 2 impls of the many dozens of impls I've written. As another example, the only method in the futures library that uses this is select.

@aturon Glad that we can still have a TLS system for the cases that want it!
I have some unresolved questions that were not touched in this thread, how much do we want to couple the Future and Stream traits to the task system? This RFC proposes a tight coupling which seems to make the Future trait essentially unusable without the task system.
I'm mystified by this sentence of the RFC:

Threading an explicit &mut Context argument makes no_std support entirely straightforward, which may also have implications for the futures library ultimately landing in libcore.

Maybe no_std integration is better for those that want the task system, but what about those who don't? How are will they get a dummy Context to satisfy the poll method? Should Context be a trait to attempt to generalize other uses cases or will there be a way to bypass it for those that don't use it?
Before this RFC, getting async/await library support stabilized seemed like a simple matter of moving Future into std or core. But if they require a Context parameter, is the task system also required to be moved to std? That doesn't seem reasonable.

With async/await using the generator syntax, the context argument would be
hidden in the generated code. How frequently do we expect users to write
explicit futures or interact with the task context when async/await is
implemented?

@erickt brings up a great point. The RFC and this discussion include very few
details regarding how the extra context parameter will interact with
futures-await. It will be necessary in certain cases to call poll inside
#[async] functions. poll is necessary when one action needs to be taken if a
futures is ready, and another action otherwise (select exists, but it's not
fully general). It appears straightforward and obvious how this will work with
the current API, but it's not clear how this could work with an explicit
Context parameter. Ideally the RFC would include a section on how the
Context parameter will affect futures-await, including modified examples (if
necessary), and keeping explicit calls to poll in mind.

Separately, as an early adopter of Futures and Tokio, and having written a few
futures-based applications including a client for a complex RPC protocol, I find
the motivation for the RFC lacking. I don't think it would be the end of the
world if the RFC were adopted, however I agree with the sentiment expressed by
many of the more experienced Futures early-adopters that this RFC is solving
non-issues in a disruptive way, ignores more fundamental hazards in the API, and
includes text which is imprecise to the point of misleading.
I'm worried that the decision making around this RFC is being greatly influenced
by folks who have never used the futures library or written an application using
async paradigms. The knee-jerk reactions of 'TLS is always bad' and 'explicit is
better than implicit' are particularly troublesome; these are easy ideas to hold
on to, but the reality is more nuanced. Many folks appear to be forgetting (or
are unaware) that Futures is not the bottom-most layer of the stack, but is
in-fact a high-level convenience API where it is entirely appropriate to provide
(zero-cost) abstractions.

@P-E-Meunier

"Large future-based codebases" will likely use combinators instead, and these won't change.

This is demonstrably false. At Buoyant, we maintain what is probably the largest future-based codebase that exists today (if this is not true, I would love to hear more about these other projects).
Starting at Conduit and spreading out across a large set of libraries that we wrote and maintain explicitly for this project (tower, h2, etc...). You can go in and measure the number of times combinators were used compared to writing functions that return Async.
I am somewhat disappointed that this RFC is based on conjecture and no effort has been done to actually measure the impact of the changes. Instead, it seems that the plan is to make a decision asap and commit to it next week with a release to crates.io.

Again, I have very little knowledge about this domain. However what @carllerche says resonates -- TLS is definitely a potential pitfal, but before making major API changes it would probably be good to do a complete overview of all major design decisions for futures and what impact they might have.
What other issues are there with futures as they exist today? Again I am just a casual observer, but I have not seen any summary of the pitfals of the current implementation.

I am somewhat disappointed that this RFC is based on conjecture and no effort has been done to actually measure the impact of the changes. Instead, it seems that the plan is to make a decision asap and commit to it next week with a release to crates.io.

@carllerche and many others:
This issue has long been a highly contentious one, and I know that passions run high, but can we please try to take it down a notch?
As should be abundantly clear from the thread, either decision here is going to result in significant disappointment for some. The futures team has been advertising this RFC widely to try to get a strong array of voices, but ultimately this is going to be an extremely hard call to make.
I don't know about any plans to release immediately to crates.io; as #1 explains, the overall goal is to coordinate with the impending Tokio release, which as I understand means about 4-6 weeks from now.  I'd appreciate if you and others would avoid making accusations of bad faith at the outset of our RFC process.
What I do hope we can do is wrap up RFC discussion in the near future and put out a finished branch that people can start testing with.

@aturon @carllerche

I don't know about any plans to release immediately to crates.io; as #1 explains, the overall goal is to coordinate with the impending Tokio release, which as I understand means about 4-6 weeks from now.

This is probably due to a miscommunication on my part-- when we discussed yesterday, I told @carllerche the plan was to release parts of futures 0.2 ASAP so that it wouldn't block the impending release of tokio. This was before the plan to release tokio 0.1 on futures 0.1 was made in https://github.com/rust-lang-nursery/futures-rs/issues/724. This new plan gives us more time to experiment and play with the new futures-rs before trying to get a crates.io release out the door.
In any case, we hope to have the futures 0.2 changes ready for people to start experimenting with sometime early next week.

@aturon I apologize if I misunderstood the intention. Based on being told "current plan is to not release separately from 0.2, with 0.2 landing next week" and this I was under the impression that the intent was cutting 0.2 to crates.io next week.
That said, I stand by my position that a change of this magnitude should have some process by which it is evaluated before committed to.
To be clear, if the decision is to switch to an explicit task argument, I plan on whole heatedly accepting it. I would just expect that this change would go through some evaluation process before committing to it.

@carllerche

I would just expect that this change would go through some evaluation process before committing to it.

Yep! I don't think that anyone wants to see 0.2 go out the door before we've had a chance to experiment with it and see the effects on users of futures.

@aturon also, to clarify, I'm not actually against this RFC. As far as I am concerned, there isn't enough information for me to decide one way or the other.
As I commented here, it seems to me that most important factor in designing Future should be how it integrates with async / await.
I would expect that a future will be implemented like this:
impl Future for Foo {
    type Item = ();
    type Error = ();

    async fn poll(&mut self) -> Result<(), ()> {
        let v = await self.inner_fut.poll()?;
        Ok(transform(v))
    }
}

And, equivalently, a free async fn would be:
async fn send_request(&mut self, request: Request) -> Result<Response, Error> {
    await self.codec.send(request)?;
    let response = self.codec.recv()?;
    Ok(response)
}

Basically, at that point, I don't care about any of the details as long the basic functionality requirements are met. If a thread-local is used or a context argument is threaded through is just an implementation detail.
Perhaps the RFC could include some of this information.

@aturon

The wrapper would provide a scoped means of installing the current task into TLS

Also, I think @llogiq 's suggestion sounds really awesome:

Make an explicit version for low-level stuff and build wrapper types that bridge them to the implicit higher level trait?

How hard would that be? I don't have a strong opinion on the proposal, but at least this would allow to experiment with both for a while to get more informed feedback. Also, this looks like it could start very soon without big changes to existing codebases (nor to futures itself, apparently).

I would expect that a future will be implemented like this

This seems unlikely. An async function is a function that return to a future, if a future is just something that is polled by calling an async function, you have a bit of a quine.
I've been working on async/await - in conversation with the futures team, but not directly coordinated with these RFCs. In my impression of where we'll end up, manual futures will need to be implemented:

for fundamental primitives (like Timer and TcpSocket)
for the combinator primitives (the most important of which are the concurrent primitives like select & join, since async/await can't replace those)

And you'll need to call poll only in an executor implementation like the event loop at the core of tokio.
So all of the code in between those - protocol implementations, actual applications, middleware - could (and IMO should) use async functions, which none of this will ultimately impact.

@withoutboats So, right now the h2 library is implemented mostly as free Async fns. This is mostly related to the fact that there are many concurrent "futures" operating on the same internal state.
Would this mean that the h2 library would see limited benefit from async / await?
I believe the same applies to tower, though perhaps it would see more gain from async await.

I suspect, based on other ecosystems, that use case would be covered by a combination of async/await and combinators. The individual concurrent futures would be async functions, and the orchestrating future would be an async function that awaits a combinator.

@carllerche Let me try to clarify what @withoutboats was saying.
async/await notation is a way of defining a future, i.e. a poll function. You wouldn't use it directly within an impl Future block, because it is producing the thing that will be the future.
You can see how this works today with @alexcrichton's crate; the final design will be along similar lines.
The free async fn definition you wrote is indeed how we expect the final system to work. It translates to a normal function as follows:
async fn send_request(&mut self, request: Request) -> Result<Response, Error>

// becomes

fn send_request(&mut self, request: Request) -> impl Future<Item = Response, Error = Error>

The question of how to handle a large set of poll-style functions that also need access to some shared state is an interesting one; it's related to parameterization of generators, which is something we plan to support, though the exact design is a bit hazy.
I think it's fair to say that a "finished" async/await story should support what you're doing in h2, and as @withoutboats says, the explicit argument introduced in this RFC has no impact on such code. In terms of shipping, generators work on nightly today, and support for borrowing across yield points is making very fast progress. The full story should be available at least on nightly by August (the epoch branch point), and stable in 2018.

We also have a large async codebase that makes heavy use of futures-rs, and a lot of it does indeed use poll directly rather than combinators.  Even with full futures-await support, I think some of our futures will still be better implemented with poll directly.  So, this change would negatively affect the ergonomics there.
That said, I think we could work through passing through Context in our poll implementations.  My main concern would be how this interacts with futures-await/generators without again needing TLS.  It'd be great to have a sketch for how this would work in this RFC.
If we're making backwards-incompatible changes, I'd also like to see more discussion around leveraging the type system to prevent errors than just improving ergonomics.  As @Diggsey mentions above, preventing returning Async::NotReady when we haven't actually parked the task would be really useful -- we've actually run into this bug in practice a bunch.

@leodasvacas Are there any examples of using Futures without the Task system, currently?

@carllerche

So, right now the h2 library is implemented mostly as free Async fns. This is mostly related to the fact that there are many concurrent "futures" operating on the same internal state.

I've had this sort of situation crop up a lot as well and have been thinking about what the transition to async/await will look like. The core transformation is reformulating functions from the form "is x signal present? Then try y action or make y state transition" to "await x signal, then do y action or make y state transition". However, this means that you're now talking in terms of blocking operations rather than a single async state machine with a bunch of possible transitions. IMO this will make the usage of the spawning and the select combinator much more common (analogously to go func() { ... } and select in golang).

@cramertj
At least today, spawning + select adds the additional overhead of requiring message passing (which also adds latency). This is why I usually try to not use it as a crutch today.

@cramertj

IMO this will make the usage of the spawning and the select combinator much more common (analogously to go func() { ... } and select in golang).

Perhaps I'm getting a little too deep in the weeds here -- but this doesn't seem like a great outcome to me.
Specifically, in Go cancellation becomes a difficult problem.  If I want to stop a go func from running, i need to pass it some channel that i can notify to cause work to stop -- and that goroutine now has to select over that cancellation channel and any other sources of work it has.
Today, to cancel a future's work, i simply drop it.  Once I've passed the future to the executor, i need additional primitives to manage cancellation.
Perhaps this fuller cancellation story has already been thought out and discussed elsewhere.
Anyway, regardless of the specifics of this one issue... Is it a broader goal of the Rust/async/futures teams to arrive on a model more like go routines with regard to adopting synchronous, procedural idioms to implement async code? I know there's work ongoing to allow this -- but is the goal for this to be the primary way to implement, for instance, an h2 codec?  If so, I'd be curious to understand more about these motivations (because I probably disagree with them ;).

@olix0r

Is it a broader goal of the Rust/async/futures teams to arrive on a model more like go routines with regard to adopting synchronous, procedural idioms to implement async code?

That's a great question! My personal take is essentially "no", in the sense that I think we want a multi-paradigmatic approach here, much like with the rest of Rust's concurrency story.
That is, it's important that it be ergonomic to use synchronous idioms if you want, but also that you retain the freedom to "drop down" to manual state machines as needed.
It's not as clear to me what, exactly, h2 might look like in an async/await world, and separately from this RFC, I think it'd be worth trying to "port" a small slice as a way of testing the async/await design. I suspect it will wind up somewhere in the middle of "sync-style" code and "pure state machine" code.
Getting back to the RFC for a moment: I don't think we should brush aside the ergonomic impact of an explicit argument under the assumption that "everyone will use async/await eventually". It's just not clear that will pan out. We should feel comfortable with the ergonomic impact on manual futures, IMO, before we commit to this.
Like @carllerche, I remain essentially on the fence about this RFC. I definitely understand the concern that a lot of the sentiment in support comes from folks with relatively little experience with futures. But there's also support from people with a lot of experience, including working on teams.
For myself, given such a close call, I do tend to give some weight to the pretty massive, consistent, and ongoing way that people have asked for this change over time.

@carllerche

At least today, spawning + select adds the additional overhead of requiring message passing (which also adds latency). This is why I usually try to not use it as a crutch today.

@olix0r

Is it a broader goal of the Rust/async/futures teams to arrive on a model more like go routines with regard to adopting synchronous, procedural idioms to implement async code? I know there's work ongoing to allow this -- but is the goal for this to be the primary way to implement, for instance, an h2 codec? If so, I'd be curious to understand more about these motivations (because I probably disagree with them ;).

I also believe that synchronous-style async/await code is not the right choice for every application for exactly the reasons you mentioned. However, I agree with @aturon that we want to allow this kind of code even if it's not the end-goal for everybody.
That said, I think there are specific ways that we could look to improve on golang-style spawn + select. For one, I think it should be possible to create some kind of select_loop macro which allows you to select over a number of futures and streams in a loop, receiving results as they become available, something like this:
async fn run(
    incoming_connections: impl Stream<Item = Connection, Error = Error>,
    change_color: impl Stream<Item = Color, Error = Error>,
    say_goodbye: impl Future<Item = (), Error = Error>)
    -> Result<(), Error>
{
    let mut connections = vec![];
    let mut color = Color::Blue;

    select_loop! {
        (connection from incoming_connections) => connections.push(connection),
        (new_color from change_color) => color = new_color,
        (goodbye from say_goodbye) => break,
    }

    let goodbye_msg = format!("Goodbye! Tomorrow, the sky will be {}.", color);
    await!(FuturesUnordered::from(
        connections.map(|conn| conn.send(&goodbye_msg))).fold((), |_, _| Ok(())))
}

This approach would allow you to avoid the overhead imposed by spawn and channels, and get instantaneous cancellation of the remaining Streams and Futures when the function returns.

Does anyone have a concrete example of some reasonable code that is better represented by explicit Future implementation than by a combination of async/await and combinators? I certainly can't think of any such in the futures code I've written, which I am eager to drastically simplify with async/await ASAP.

@Ralith I've written a number of things that process many possible incoming events in the same poll fn. In those case, I'd need something like the select above to make it doable with async/await.

@cramertj That looks equivalent to a chain of select combinators to me. Of course, the macro is much more ergonomic (though I don't know that it needs to have a loop hardwired in). Variadic generics and structurally-typed enums could someday allow an elegant macro-free solution, but regardless it doesn't seem like a strong case for writing Future impls by hand unless you're implementing a leaf, a combinator, or a combinator-like macro.

@Ralith

Does anyone have a concrete example of some reasonable code that is better represented by explicit Future implementation than by a combination of async/await and combinators

I'm not sure this counts as better represented, but it is frequently necessary in order to avoid heap allocations.
Something I've encountered many times: let's say you can do something like foo().and_then(|f| bar(f))... but what is the resulting type of this?  It's a AndThen<Foo, Bar, ...something?>  So if, for instance, i want to store a Vec of these... what type is that Vec?  I can't know the concrete type of these, and so my options are either to put it in a Box or to implement a future manually....
Is this boiler plate annoying? you betcha. But, especially when buildings libraries, it's extremely desirable to avoid these heap allocations; and to do this, we generally have to hand-roll our futures.
If someone's proposing a world where i can use combinators in library code without resorting to heap allocation, sign me up!

@olix0r Those problems should be resolved by RFC 2071, which will allow you to give names to complex or unnameable types.

@Ralith

That looks equivalent to a chain of select combinators to me.

I'm not sure how you would represent my example using select as it exists today without resorting to heap allocations.

@cramertj Reviewing the detailed types, it's apparent that this using combinators would be tedious and ugly, but combining Stream::into_future, Future::select2, and perhaps the impl of Future for Option<F> where F: Future should allow you to create a future yielding a distinguishable item from any of the three sources along with all the pieces necessary to reconstruct a similar future, suitable for feeding back into a loop_fn iteratively.

I think it's pretty clearly desirable to be able to select between a fixed number of futures and streams, in an async function, without heap allocation. It shouldn't have to be a reason to drop down to manually implementing poll.

@rpjohnst Absolutely! I brought it up because I think it's one of the main places where it's confusing and difficult to use combinators or async/await today as opposed to writing manual future impls.

These points have probably been touched upon before, so I'll be quick with my two cents on the ergonomics around learning and understanding futures code with an explicit task::Context:

When learning futures for the first time, having an explicit task::Context represents a known-unknown that the user may need to understand at some point (and being part of the API makes it difficult to miss, even when ignoring the the docs).
Relying on the more implicit TLS implementation is an unknown-unknown: unless they encounter an explanation in the docs or see it being used, the user may miss the notify component completely and stumble in confusion
In the end, the user still needs to become familiar with the same concepts, but I believe the former method has a better passive discoverability.
I've also repeatedly seen discussion and confusion around determining if a future is running on a task or not (example rust-lang-nursery/futures-rs#691). Assuming that a task::Context can't be accessed outside of a running task, just having an instance of it available is a good guarantee that we won't see a panic from polling off task

I have a futures-heavy crate that defines "context aware" future combinators which require threading some environment through each call to poll (example). IMO the impact of passing a variable around between polls is negligible.

For my own edification, is there anything about rust or the futures library in particular that mitigates or fixes the usual downsides already expressed associated with using TLS for things?
My—admittedly non-futures-rs—experience with TLS has always been that it feels great for a bit, but then there's that 0.01% bug where things just don't quite work right, and some combination with another library using TLS that turns out to just not work at all.

@Ralith

Does anyone have a concrete example of some reasonable code that is better represented by explicit Future implementation than by a combination of async/await and combinators?

Some network protocols are better expressed as state machines with gotos, than as combinators. SSH is an example I'm familiar with (see the Thrussh crate).

I can't find Alex's introductory post, but one of the things it focused on was explaining why futures in Rust used a poll mechanism instead of a callback mechanism. One of the things that seemingly never got explained is that while the star of the show is the poll function, that you can't usefully call the poll function and if you do directly, you get a panic. The TLS based task system has always seemed hidden and I don't see why that was necessary or beneficial, or why perpetuating it would be a good idea.

Given that one day Rust might replace C for low-level/embedded stuff on strange processors (e.g. using a future GCC front-end), requiring something as complex as TLS as a dependency for something as fundamental as a Future seems like too big a risk to me. IMHO fundamental things should ideally be minimalistic and simple (inside as well as outside), to avoid the complexity of the system multiplying as layers are built above them.
(GCC docs on TLS note that "It requires significant support from the linker, dynamic linker, and system libraries, so it is not available everywhere." Some more info on the internals of TLS for ELF: https://www.akkadia.org/drepper/tls.pdf TL;DR, loading shared objects makes this much harder than it might appear)

Is there a fork of futures-rs/tokio/etc which works and is implemented using a context parameter? Would creating a fork to get experience be beneficial?

I fully agree with the learnability argument given here. I should mention that I'm not actually a user of Futures/Async, but I was very curious how Rust would approach this and so closely followed the original blog posts when all this infrastructure was first announced. They left me very confused, I just couldn't understand how this would work -- how would the event loop know to wake up certain tasks when certain file descriptors have data on them? It took a while until there was a blog post explaining this, and even then I still recall asking @aturon about this in person before fully grasping what went on.
Most Rust programmers are very cautious about global state due to static mut being unsafe. I think this is a great lesson to learn, and I'm happy to see the singleton (anti-)pattern used much less in Rust than elsewhere. Thread-local state is hidden global state just as much as static mut is, it just happens to be safer -- but in terms of reasoning, or understanding which inputs go into a computation and which effects it can have, TLS is just as bad as global variables. I think the API becomes much better understandable by no longer relying on a per-thread singleton.
(I wouldn't usually have commented, but @aturon was asking for more comments, so here you go. ;)

I'd like to add that what I call semi-global state is not uncommon and people are used to it. A classic example being web frameworks that have to carry the request/response context through the whole request. Other server systems have similar issues.
People coming from these areas rarely see a problem in these kinds of approaches.

I in fact am an author of such a package: https://rubygems.org/gems/request_store
Ironically, I created it in order to solve problems with TLS; that is https://github.com/steveklabnik/request_store#the-problem

People coming from these areas rarely see a problem in these kinds of approaches.

As someone who has been forced to learn PHP at work, while you seem to be correct in that there are people who don't see the problem, I've seen the problem and had to work around this aspect of PHP. In the end, different languages encourage different patterns. Rust clearly doesn't encourage this particular pattern, or even make it easy to do.
Code should be idiomatic to the chosen language. In Rust, that should be things passed by parameter (in this case) and only hidden in TLS under very special circumstances.

Ironically, I created it in order to solve problems with TLS; that is https://github.com/steveklabnik/request_store#the-problem

Jep, with that in mind, I specifically avoided mentioning thread local storage specifically for that reason, (which is a very different beast from Task/Request Local Storage).

As someone who has been forced to learn PHP at work, while you seem to be correct in that there are people who don't see the problem, I've seen the problem and had to work around this aspect of PHP.

PHP has very interesting use of global state, which is very different on what many web frameworks use.

Code should be idiomatic to the chosen language. In Rust, that should be things passed by parameter (in this case) and only hidden in TLS under very special circumstances.

I avoid calling any Rust code idiomatic. The language is not very old, and whether implicit task-local passing is a convenient thing has to be determined. Shipping a Context through a whole request chain is extremely tedious and also means that anything involved in that chain always have to deal with the context, even if it doesn't touch it.

I remember @japaric mentioned the possibility of someday using futures or async/await in his embedded projects ('realtime for the masses'), and haven't seen him in these threads yet, maybe he might have some thoughts?

I'm using futures with async/await in my embedded project. It is perfectly doable using task::init(), however I'm in favor of explicit context. One specific use case is a no-op notifier. A future is served by an interrupt, and a future executor is running in the main thread. The executor polls the future in a loop, and calls asm!("wfi") (wait-for-interrupt instruction) on Async::NotReady. This way no action for notification is needed, the executor thread wakes up by hardware on any interrupt. If we would have an explicit task argument, I believe that the whole task system could be optimized away.

A classic example being web frameworks that have to carry the request/response context through the whole request. Other server systems have similar issues.
People coming from these areas rarely see a problem in these kinds of approaches.

I'm a counter-example, then, as problems with contexts in web stacks is exactly where my biggest negative experiences with TLS have come from.
It seems these contexts are becoming rarer, too, as new things like ASP.Net MVC make the request a field on the controller, for the 90% case of using the request in private helper methods there, but require that you pass it around explicitly outside of that.  (Conveniently, this improves testability too.)

It's also worth noting that scoped TLS is incompatible with stackful coroutines: even if you don't plan on using them together, it means that just having an indirect dependency on both futures and a stackful coroutines library breaks rust's safety guarantees.

@yodaldevoid it seems @valff has an example. @valff currently if don't need the task system then you simply don't use it and it won't create any cost for you. This RFC creates the need of passing a no-op context, thought how that is possible is not described in the text.

tl;dr - possible solution to passing arguments everywhere, but regardless, now in favor of RFC.
My main worry:
"Having it as an argument aids learnability." Not necessarily. It front loads a new concept immediately to a new user, who may not need to know what it does. If the answer for new users is "don't worry about it, just pass it through to the futures you've wrapped", then a user is just left with the impression "why do I even need this thing then?".
Look at Go's context appearing everywhere. You have users (I was one) that just pass context.Background() as an argument to make the compiler happy, not really caring what it is.
My main worry is that async code in Rust becomes like Go, where everyone passes around a Context argument, not necessarily sure why, grumbling about where to get one in tests. So, I've been thinking about this problem the past couple days, and here's what I got.
Solution to argument passing proliferation
In general, I'm now in favor of this RFC, because of these reasons:

Passing an argument is more idiomatic in all other circumstances.
Those of us who dislike it, can create patterns on top, like the TLS that exists in 0.1.
I may have a suggestion that's better than both explicit argument, and TLS; see below.

Arbitrary Self Type: WithContext
With arbitrary_self_types, we can decorate our own types' methods with a special smart pointer to allow passing a context into the method without having to pass another argument, which is especially useful when you start to have several arguments (more than 5ish).
Imagine that a "smart pointer" WithContext were written, which bundled together this &mut Context with any value. Essentially, this:
pub struct WithContext<'a, T> {
    ctx: &'a mut Context,
    val: T,
}

impl<'a, T> Deref for WithContext<'a, T> {
    type Target = T;
    fn deref(&self) -> &T {
        &self.val
    }
}
// impl DerefMut

With this "smart pointer", any internal type can add a method saying that it needs a bundled context to call it:
impl Streams {
    fn send_headers(mut self: WithContext<&mut Self>, stream: Ptr, headers: Headers, eos: bool) {

    }
}

The above method cannot be called with simply a &mut Streams, but must be called with a WithContext<&mut Streams>.
This WithContext could be done as an external library, but if it were available in futures directly, we could extend Context, making it nicer to use:
impl Context {
    pub fn with<T>(&mut self, val: T) -> WithContext<T> {
        WithContext {
            ctx: self,
            val: val,
        }
    }
}

The way to use this, then looks like this:
impl Future for SendHeaders {
    fn poll(&mut self, ctx: &mut Context) -> Poll<Self::Item, Self::Error> {
        // check state
        // ...
        ctx.with(&mut self.inner.streams)
            .send_headers(...)?;
        // other state
    }
}

This actually has some pros over the TLS version, as well, by statically requiring that methods that need access to a context are passed one.
Alternative (or rather, Additionally)
If fleshed out more, this could possibly even be used for the Future trait directly, like:
trait Future {
    fn poll(self: WithContext<&mut Self>) -> Poll<...>;
}

But I haven't explored this design enough to fully recommend that yet.

Now I'm interested in this pattern outside of futures, heh! very interesting!

I avoid calling any Rust code idiomatic. The language is not very old, and whether implicit task-local passing is a convenient thing has to be determined.

If you're suggesting that passing data to functions using parameters isn't idiomatic in Rust, even at this 'young' stage (of more than 10 years at this point), then I don't know what to say.

Shipping a Context through a whole request chain is extremely tedious and also means that anything involved in that chain always have to deal with the context, even if it doesn't touch it.

You could replace Context here with Lifetime and your statement would be just as true, and it's a huge criticism of Rust by people who write 'likely' incorrect code in other languages. Clearly there are cases where the effort is worth it.

Arbitrary Self Type: WithContext

Using a feature that isn't stable yet to implement what will likely be one of the most prevalent libraries in the Rust ecosystem just to avoid passing a parameter to a function seems like a bit much. It seems technically cool, though, I'll admit.

Using a feature that isn't stable yet to implement

The good news is, if you check that tracking issue, that allowing smart pointers to be used on non trait-objects seems much close to stabilization!

@seanmonstar That's a very clever approach, well done!

Sounds neat! I'm not sure I understand the advantage of the arbitrary_self_types approach vs just offering a poll method directly on WithContext<T: Future> that delegated to the underlying poll with the given context. Mind explaining? Is the point that methods other than poll could also pick up the context?

@cramertj I don't quite understand your question, either :D
The point is that the thing that needs to take task::current() (in this RFC, context.waker()) may not be directly in the <T as Future>::poll method, but several, several layers deep. This is the case in h2, where only after passing through a bunch of methods, may the internal state machine need to take the a waker. The send_headers function in my example was simplified from h2. So, we would either have to pass this context argument around, or we could put the context in the receiver part instead.
Also forgotten in my example is a way to grab the &mut Context from a WithContext, but expect there'd be a simple getter method too. So, in fn send_headers, it access itself, and the context, from self:
fn send_headers(mut self: WithContext<&mut Self>, ...args) {
    let buf = frame.into_buf();
    // call a method of `Self`
    let n = self.write_until_blocked(&buf);
    if buf.len() > n {
        // get the `Context` from `WithContext<Self>`
        let waker = self.context().waker();
        // set some properties on `Self`
        self.partial = Some((buf, n));
        self.waker = Some(waker);
        Ok(Async::Pending)
    } else {
        Ok(Async::Ready(n))
    }
}

@seanmonstar Oh, so the point is that you want to be able to have the context available when calling from one method on type Foo through a chain of methods, all on the same Foo? That makes sense. Sorry, I misunderstood. Thanks for explaining!

@cramertj What he means I think is that a Context will have to be plumbed down through many poll calls until it hits a future that will actually does something with it. By stashing it in self it gets plumbed down through self which is more ergonomic.

@leodasvacas In that case it allows you to write

poll() instead of
poll(ctx)
and
fn foo(mut self: WithContext<&mut Self>) instead of
fn foo(&mut self, ctx: &mut Context).

To avoid the "Return Async::NotReady without calling notify() first" problem, you can do the following:
// ZST
struct TaskNotifyHandle {}

impl Task {
    pub fn notify(&self) -> TaskNotifyHandle {
        self.events.notify();
        self.unpark.notify();
        TaskNotifyHandle {}
    }
}

pub enum Async<T> {
    /// Represents that a value is immediately ready.
    Ready(T),

    /// Represents that a value is not ready yet, but may be so later.
    NotReady(TaskNotifyHandle),
}

Note:

You need to make sure TaskNotifyHandle can only be constructed by Task. This might require adding a zero-sized private field inside it (the type itself needs to be public)?.
Async now effectively depends on Task. This seems reasonable?
Open question: Should TaskNotifyHandle do something? It could use drop to unregister whatever notifications were initially registered as a perf optimization. It could be used to remember which future is waiting on what, to make select and counterparts more efficient. Not clear if that's worth the complexity increase.

As to the general question of learnability/safety, I not too long ago helped someone struggling with poll: https://www.reddit.com/r/rust/comments/7jvzvy/new_to_rust_and_getting_crash/
It's a good example of the implicitness hazard, the user never noticed there was an issue with their code until they ran into a crash.

The good news is, if you check that tracking issue, that allowing smart pointers to be used on non trait-objects seems much close to stabilization!

I think you misunderstood my point. One of Rust's driving principles is to base the language on well understood techniques. If one were to generalize this, it would suggest a conservative approach where there isn't significant benefit from being aggressive.

@seanmonstar

The good news is, if you check that tracking issue, that allowing smart pointers to be used on non trait-objects seems much close to stabilization!

Hm, I checked the tracking issue, and it says "This needs an RFC before stabilization...". And no RFC has yet come out of it.
That doesn't see much close to stabilization.
I have to say though, that this is a very neat way to carry context around.

One issue WithContext might be susceptible to is that we've by convention avoided putting any inherent methods on types with Deref impls due to the silent shadowing/fallback between inherent and through-autoderef methods, which is invisible at the call site. Hence e.g. we write Rc::make_mut(x) rather than x.make_mut().

We can definitely increase the priority of the self types feature; it's also an important ingredient for the async/await work. Also, this is one of those cases where the RFC would be essentially "for stabilization", so we should be able to move quickly. Seems fine to rely on, to me.
@glaebhoerl Indeed, but I suspect that WithContext would only want to provide a couple of very well-known methods (for getting the context, etc).

So I just thought about this, and I want to check if it's a thing or not. If we pass the equivalent of &mut Context, will we have issues implementing catch_unwind?
I've never dealt with panic safety before, so I'm not sure if this prevents us from being so (we have &mut T, and RefCell for local storage). Is the current TLS context even strictly speaking panic safe?

Given the current state of the thread -- and especially @seanmonstar lovely insight -- the futures team is inclined to merge this RFC.
So of course, now it's time for some old fashioned 🚲 shedding!

task::Context
task::TaskContext
task::TaskCtx

@aturon You left out task::Ctx :stuck_out_tongue:

some old fashioned 🚲 shedding

I figure this would go in the futures prelude, alongside Poll and Async?  To me, that argues for a full TaskContext, since just Context is so incredibly unspecific.

@ahmedcharles:

I think you misunderstood my point.

Hm, even with the following sentences, I still must be misunderstanding, sorry!

@skade:

"This needs an RFC before stabilization...". And no RFC has yet come out of it.

True, but I bet stabilizing just a part of the full feature, allowing self: T where T: Deref<Target=Self> on inherent methods, and deferring the other more complicated questions, could be done quickly. Work on the specific subset of the feature has seen progress recently specifically because async/await suggests that #[async] functions might want to be self: Rc<Self>.

@glaebhoerl:

[...] we've by convention avoided putting any inherent methods on types with Deref impls due to the silent shadowing/fallback between inherent and through-autoderef methods [...]

That's true. My personal opinion is that a single method like context should be pretty clear, and people won't assume it's calling their own fn context(&self) method. If it were a big deal, though, we could just make them be associated functions instead, like Rc and Arc.
We may even want extra functions besides just context, and those maybe should be associated functions:

WithContext::split(ptr: WithContext<T>) -> (&mut Context, T) - To allow wrapping an inner field in a context, while still having mutable access to all other fields, for instance.

@aturon:

Given the current state of the thread -- and especially @seanmonstar lovely insight -- the futures team is inclined to merge this RFC.

Just curious, is there any thinking around my suggestion to maybe include it with futures, or is the preference that this be an external crate? I don't know if it should actually be called WithContext, or something else, it was mostly a name to just get the suggestion written up.

@scottmcm

I figure this would go in the futures prelude, alongside Poll and Async? To me, that argues for a full TaskContext, since just Context is so incredibly unspecific.

That seems reasonable to me. i suppose you could also include the task module in the prelude directly so that you get task::Ctx, or you could do pub mod prelude { pub use task::Ctx as TaskCtx; }.

My personal opinion is that a single method like context should be pretty clear, and people won't assume it's calling their own fn context(&self) method. If it were a big deal, though, we could just make them be associated functions instead, like Rc and Arc.

Since retrieving the context itself should be fairly rare there is probably not a lot of harm in making the function(s) interacting with the context associated functions.

Using WithContext as the receiver for poll doesn't seem fruitful. Any nested future won't be held as a WithContext<T>, but just as T, so you'd have to reapply the context every time you want to poll the underlying future.

@seanmonstar

Just curious, is there any thinking around my suggestion to maybe include it with futures, or is the preference that this be an external crate?

So, couple things here:

I think ultimately, if this pattern catches on, we might want to put this in std.
For Future::poll, I think at least in 0.2 we should stick with an explicit argument. We can potentially reconsider that when we have more experience (and the feature is stable).

Note that we expect at least two more major releases of the core futures crate this year:

0.3 for adapting the async/await changes, whatever those turn out to be
1.0

so we'll have further opportunity to revise.

@christophebiocca

To avoid the "Return Async::NotReady without calling notify() first" problem, you can do the following...

Indeed! There are even some further ideas along these lines that could allow for a better debugging experience.
However, note that this only helps ensure that you've scheduled yourself somewhere, but not necessarily all the places you need to be scheduled.
My feeling was that we should punt on this question right now, but experiment with it and consider it for 0.3.
@alexcrichton @cramertj thoughts?

@aturon

For Future::poll, I think at least in 0.2 we should stick with an explicit argument.

Agreed, especially since we want to land 0.2 relatively soon (before arbitrary self types are stable).

However, note that [a zst added to Async::NotReady] only helps ensure that you've scheduled yourself somewhere, but not necessarily all the places you need to be scheduled.

@carllerche and I discussed this pretty extensively-- personally, I'm opposed to this idea because it further confuses the case that I already consider to be the most confusing manual future impls: selecting over several child futures. In this case, you only want to return NotReady when all of your children return NotReady. However, in order for the ZST pattern to work, you have to pick a single NotReady to return back up, which feels confusing and unergonomic to me.

My feeling was that we should punt on this question right now, but experiment with it and consider it for 0.3.

Agreed as well!

Hm this will probably work better if I put forward a "leading" suggestion and people can object: TaskCtx. Any strong objections?

@aturon, @cramertj now that a context param has been accepted, is there room to discuss if it should be an associated type? So it could be type Context = TaskCtx; or type Context = ();
I've personally found it confusing to know when a Future is associated with a task (and needs to be driven) or not. There is at least one more issue, with different motivation, asking for something similar.
Example where a future would not be associated with a task:
let future: impl Future<..., ..., ()> = thread_pool.execute(|| {
    // Expensive calculation or disk io.
});

@yazaddaruvala Futures are always associated with a task, and always need to be driven. The example you gave is no exception.

So of course, now it's time for some old fashioned 🚲 shedding!

So, I went to https://rust-lang-nursery.github.io/api-guidelines/naming.html to look for the rule, but it wasn't there!
However, for the stdlib at least, http://rust-lang.github.io/rfcs/0356-no-module-prefixes.html suggests that task::Context is appropriate.

@Diggsey I'm not sure I understand this. A thread is driven to completion by the operating system. Regardless of my using future.poll() the operation will execute to completion.
Maybe I'm missing something here (would love to learn) but there is no "pausing" of a thread (from the application's perspective) and therefore no reason to notify() anything. Why would there be a task associated, and even more what use would a TaskCtx be to me in my example?

@steveklabnik Having written that RFC, I can say that these days the Libs Team regrets the blanket prohibition. (I've even come to wish we had done IoResult etc). Precisely because these types often end up being use out of... context ;-)

@yazaddaruvala

what use would a TaskCtx be to me in my example?

You might want to use it to store some task-local data or access other fields or methods on TaskCtx. @aturon is currently working on a separate RFC that would add other functionality to TaskCtx (functionality which would be desirable even in a blocking context).

@cramertj That makes sense! But in order for me to store some task-local data, shouldn't context be an associated type or be generic in some way?

@yazaddaruvala Task-locals are currently implemented using the task_local macro, which relies on a task-local HashMap to store the data.

@yazaddaruvala I experimented previously with making TaskCtx a generic parameter, because it does allow you to do some cool stuff, like for (the old verison of) tokio you wouldn't have to thread through a "Handle" anymore, it could just be accessible from the context.
However, it results in a kind of generic parameter explosion, and I think in the end the type safety (ensuring some futures could only be spawned to certain executors at compile time) was not worth the cost.
You can get the same functionality (albeit without the compile-time guarantees) by using an AnyMap/TypeMap on the task context, exactly the way that iron stores request context.

I rather like to reduce stuttering, and have no problem with import a module to use types with possibly ambiguous names. Here's how I always implement a Display:
use std::fmt;

impl fmt::Display for Foo {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        // ...
    }
}

Applied to futures, it could something like:
use futures::{task, Future, Poll};

impl Future for Foo {
    fn poll(&mut self, ctx: &mut task::Context) -> Poll<...> {
        // ...
    }
}

Any who don't want to scope by module can do a rename on import:
use futures::{
    task::Context as TaskCtx,
    Future,
};

@seanmonstar The point about the prelude here, though, is pretty strong.

@aturon I'm actually in another camp, then, because my personal opinion is that use *, even with preludes, is a code smell. I definitely wouldn't want to encourage it, but I wouldn't want the existence of such a pattern to negatively influence other patterns.
(I suppose the prelude module could be the place to re-export it with a new name...)

However, note that [a zst added to Async::NotReady] only helps ensure that you've scheduled yourself somewhere, but not necessarily all the places you need to be scheduled.

@carllerche and I discussed this pretty extensively-- personally, I'm opposed to this idea because it further confuses the case that I already consider to be the most confusing manual future impls: selecting over several child futures. In this case, you only want to return NotReady when all of your children return NotReady. However, in order for the ZST pattern to work, you have to pick a single NotReady to return back up, which feels confusing and unergonomic to me.

A clean way to fix that problem is to introduce Async.or_else(F) which works exactly like Option.or_else and other analogous helpers (FromIterator?). It's still possible to forget to call the child futures at all, but it does fix the combination ergonomics.
I'm bringing this up because it also breaks backcompat, which means it might be worth doing at the same time as this. If futures is going to have a 0.3 before going to 1.0 then this can be discussed separately later.

consider to be the most confusing manual future impls: selecting over several child futures. In this case, you only want to return NotReady when all of your children return NotReady. However, in order for the ZST pattern to work, you have to pick a single NotReady to return back up, which feels confusing and unergonomic to me.

A clean way to fix that problem is to introduce Async.or_else(F) which works exactly like

I would rather say Async.and(future.poll()) which is also a common combinator, but the idea is the same.
And I want to second that this is more important thing than explicit/implicit context in my opinion (as forgetting wakeup events have already beaten me in production). So experimenting with it earlier might be important.

@aturon

@seanmonstar The point about the prelude here, though, is pretty strong.

Why not have the prelude reexport the task module, allowing us to standardize on task::Context? I don't see why it would have to specifically export the type directly.

abbreviations

Please no.  My brain isn't smart enough to remember that it's TaskCtx but TyCtxt but 'tcx...
I would have no problem with task::Context being exported in the prelude as TaskContext, though.

@cramertj

@carllerche and I discussed this pretty extensively

Yes, but I did not come to the same conclusion. I was of the opinion that this would definitely be worth exploring.
As mentioned above, implementing select doesn't actually have to be hard. Assuming a join:
fn poll(&mut self, ctx: task::Context) -> Poll<...> {
    // Either start w/ not ready or you have to do a reduce style op.
    let mut ret = ctx.not_ready();

    for child in &self.children {
        match child.poll()? {
            Ready(_) => unimplemented!(),
            NotReady(v) => ret.join(v), // naming TBD, others have proposed alternatives
        }
    }

    Ok(ret)
}

As I mentioned in IRC, there is a potential for significant debug-ability wins. In debug mode, the value does not have to be zero-sized.
One idea I had off the top of my head was including a task identifier that returned the NotReady. That way, you could catch the bug where you save off a Wakeup and don't update it when the future being polled moves to another thread.

@carllerche

As mentioned above, implementing select doesn't actually have to be hard. Assuming a join: ...

I'm not sure what sort of code you're imagining going in unimplemented, but returning Ok(NotReady) at the end without ensuring that -all- of the children have returned NotReady first means that you will be missing wakeup signals. In order to solve this, you can put the whole thing in a loop which only breaks if not a single child returned Ready in the current iteration. For your join operation to be correct, it has to be reset at the beginning of said loop (since the previous iteration's NotReadys are most likely not going to count). This isn't undoable by any means, but it's certainly introducing significant friction.

As I mentioned in IRC, there is a potential for significant debug-ability wins. In debug mode, the value does not have to be zero-sized.

Yeah, as we discussed I think this is a super interesting idea and it could be really cool! I personally think this wakeup-debuggability idea is a much stronger argument for a NotReady ZST than the learnability/error-preventing one. Still, I have a hard time imagining how such a thing would work in practice (i.e. how you'd pragmatically compare the expected vs. actual wakeup Task sets and use that to figure out which Future impl was missing its registration), and I'm not entirely convinced it's worth the additional friction. I'd be more than happy to be proven wrong here, though!

So I'm playing around converting the code base, and I have hit a roadblock with trying to implement catch_unwind (See comment). Does anyone know how to work around this?

@udoprog Note https://github.com/rust-lang-nursery/futures-rs/pull/744
We should probably coordinate to avoid duplicated effort, but I just noticed that as well.

@ahmedcharles that's Great! I started this to get an idea of the impact, and it's not a ton of work. I don't mind duplication here :). This seems like a change much better executed in a place and the rest reviewing.
I've echoed the question in the PR. Please respond there!

Having written that RFC, I can say that these days the Libs Team regrets the blanket prohibition. (I've even come to wish we had done IoResult etc). Precisely because these types often end up being use out of... context ;-)

Oh wow. Guess my taste is entirely opposite of the libs team. Looks like I'll be the one doing a lot of importing with as, then 😉 😆

+1 for me on task::Context. I think the prelude is a code smell as well. If we must do it, then I think reexporting task is the best way to go.

I'm also +1 on preventing what I think of as "faux namespaces" (I think this comes from the Python community?). Also note that working around this naming issue is somewhat asymmetric:
// renaming namespaced to prefixed
use foo::Context as FooCtx;
// renaming prefixed to namespaced
mod foo {
    pub use foo::FooCtx as Context;
}

So, I think preventing faux namespaces makes sense in part because, if you want to, doing it as an API user is easy.

Maybe I'm missing something here, but shouldn't Context be a type parameter of the trait? As far as I understand, different reactors might want to implement it differently.

For those of you voicing opposition to preludes: how would you like the trait imports to work? Do you want to use futures::{Future, FutureExt, Stream, StreamExt, Sink, SinkExt}; in order to have all of the combinators available? If the answer is "yes", that's fine, but it seems rather unergonomic to me.

@Kixunil It references a trait object internally.

Yes, I think writing a slightly longer import once is better than the cost of every time I come back to the file, or show up to a new one, and wonder where something came from, and then look at the top and see a bunch of glob imports, and then just nope back out of the file.

At least in Java-land, * imports are discouraged, and your IDE imports things for you and often folds the import block away by default.

@sfackler

your IDE imports things for you

Yeah, that's a key point we're missing that makes a significant difference in ergonomics.

@cramertj why not pass around trait object itself, then? I was also thinking whether dynamic dispatch would hinder performance, but I guess it's mostly used for slow path, so that should be fine.
Speaking about IDEs, maybe that's the answer to our ergonomics problem regarding passing context around: create some standardized marker for IDEs to know that they should auto-fill that argument.
Something like this:
trait Future {
    // type ...

    fn poll(&mut self, #[auto_pass] context) -> Poll<Self::Item, Self::Error>;
}

the IDE would detect all instances where the trait is used and show it like this:
fn poll(&mut self, _) -> Poll<Self::Item, Self::Error> {
    self.foo.poll(_)
}

Speaking about IDEs, maybe that's the answer to our ergonomics problem regarding passing context around: create some standardized marker for IDEs to know that they should auto-fill that argument.

At least from my IDE experience, that's a too complex feature for IDEs to make it work reliably and not get in the way :) That is, it is too complex from the "how do we expose this to the user" side, not from "that's to hard to implement" side.

@matklad the same way IDEs can collapse blocks of code, they could collapse variables. It's just horizontal, instead of vertical. :)
IDEs should be able to know about the traits thanks to RLS.

@cramertj would it be possible to extract the task local proposal to a separate RFC? I think there is more to discuss there but it has been lost in the noise.

@seanmonstar:

That's true. My personal opinion is that a single method like context should be pretty clear, and people won't assume it's calling their own fn context(&self) method. If it were a big deal, though, we could just make them be associated functions instead, like Rc and Arc.

@aturon:

Indeed, but I suspect that WithContext would only want to provide a couple of very well-known methods (for getting the context, etc).

Speaking as someone new to Rust, I'd find this inconsistency startling in practice. Operations that manipulate the pointer should be distinct from operations that act through the pointer. In C++, for example, the methods that act on smart pointers use structure references (.), while methods that act through the pointer use pointer dereferences (->). As I understand it, the respective patterns in Rust are methods and associated functions.
That this is enforced by convention, instead design, does give you the flexibility to implement a competing pattern. Without a clear and compelling reason to break from the convention, however, I'd be wary of doing so. I haven't seen such a reason. To the contrary, your responses concede @glaebhoerl's argument, indicating the decision to break with convention is arbitrary.
Have I missed something?

@zaaq:

To the contrary, your responses concede glaebhoerl's argument, indicating the decision to break with convention is arbitrary.

I can say that personally, it's because I don't particularly agree with the convention.
For instance, if I wrap something in an Arc or an Rc, I know that I can clone the pointer when I need more references. However, thanks to a (IMO ill-advised) clippy lint, people are changing my_thing.clone() into Arc::clone(&my_thing). I find that noisy and unnecessary, since it's at least obvious to me that calling clone on Arc<MyThing> is cloning the Arc.
Now, some of the other part of the convention, like make_mut, were partly made as associated functions in order to not break existing code. They were adding new methods to a smart pointer that already had existed for a long time, and so any code that had MyThing::make_mut(&self) being called from Rc<MyThing> could suddenly cause compilation errors, so it was forced into the convention.
In this case, the smart pointer is new, and so doesn't have to worry about breaking existing code. And I'm convinced that self.context() where self: WithContext<Self> is obvious. I think it would just serve to be annoying if everyone had to write WithContext::context(&mut self) instead.

My biggest hestiation of WithContext is that (unless I am missing something, which is entirely possible) it makes it noisier to call a fn on a field. Would you have to do something like:
fn foo(self: WithContext<Self>) {
    self.map(|s| &self.inner).foo_inner()
}

which seems like more effort than just passing an argument around.

Yes, you would need to do some sort of mapping to call methods of fields. I mean, in the end, this is just a compromise since others really wanted explicit arguments and I didn't want to die on this hill.
It does have benefits when calling multiple methods of self that all would have wanted access to context, though.

Huzzah! This RFC has been merged. We'll start off by introducing task::Context, and we can iterate further on the importing ergonomics as necessary.
@carllerche I've added an unresolved question about the task-local accessors and opened https://github.com/rust-lang-nursery/futures-rs/issues/753 to follow-up.

@udoprog

I've never dealt with panic safety before, so I'm not sure if this prevents us from being so (we have &mut T, and RefCell for local storage). Is the current TLS context even strictly speaking panic safe?

Sorry to answer your question so late, but: no.  TLS (in Rust as it is currently implemented) isn't generally panic safe and can't be detected as such.  It's an unfortunate tradeoff, but it's one we made primarily because people don't use TLS in Rust very much... which, not to beat a dead horse, is one of the reasons I'm glad this RFC was accepted :)