Repository navigation
RFC: document a strict Future contract that forbids snoozing - #4019
oconnor663 wants to merge 5 commits into
Conversation
Require a future's owner to poll it when it requests a wakeup, to avoid deadlock bugs like ["Futurelock"]. This RFC is a docs-only change, but this will have far-reaching consequences for a lot of common async patterns. RFC TODO is being submitted at the same time. It adds `poll_progress` to `AsyncIterator`, in part to satisfy the strict `Future` contract introduced in this RFC. ["Futurelock"]: https://rfd.shared.oxide.computer/rfd/0609
|
|
||
| [cancelling_async_rust]: https://sunshowers.io/posts/cancelling-async-rust/ | ||
|
|
||
| ## Rationale and alternatives |
There was a problem hiding this comment.
Locking primitives could drop their reservations if they are not polled after wakeup. They haven't emitted Poll::Ready to return the lock guard, so the lock is preemptable.
In practice this is impossible without some new "WakeToken" API, where the token must be delivered to the Future that woke up. Otherwise, dropping the token indicates that the future wasn't polled. Mirroring wakers, there could be a "RawWakeTokenVtable" with a consumed and canceled callback, etc.
However I think it is far too late to have something like this but I'd love for this to be proven wrong :)
There was a problem hiding this comment.
They haven't emitted
Poll::Readyto return the lock guard, so the lock is preemptable.
If I understand you correctly, I think that's true in the concurrent streams example deadlocks, but not in the others. In the others, the snoozed future holds an actual lock guard, not just a spot in the queue. Footnote 11 also has a playground example of a concurrent streams deadlock with an unfair mutex, where there is no queue, to emphasize that lock fairness isn't essential to the problem.
There was a problem hiding this comment.
This makes a lot of sense to me! It seems like the futures ecosystem has a liveness problem. Rust is great for reasoning about safety properties, but we have a bit of a blind spot when it comes to liveness. (C++ calls this "forward progress" and they think about it a lot. I do not agree with all their takes on it, especially with how they sometimes needlessly conflate it with UB, but they have clearly recognized the importance of this topic.)
The first step in fixing that is to clearly document the intended contract: who is responsible to ensure which kind of progress? We cannot start fixing bugs if we cannot even pinpoint which component is buggy. I do not have the expertise to say what would make a good liveness contract for Future, but the proposal sounds plausible and it is not clear what the alternative even could be. It'll be a long and painful transition to get every component to adhere to the contract, but fixing the liveness problem has to start somewhere.
We should however also make it clear that failing to uphold the liveness requirement can at worst lead to deadlocks. It is not UB to fail to call poll again. The system must remain safe even if liveness is broken.
| What these cases have in common, though, is that barring program exit or the | ||
| power going out, future progress is guaranteed. There's no legitimate way for | ||
| user code to stop the runtime from working through its task list or freeze a | ||
| private worker thread.[^illegitimate] The deadlocks above are different: a | ||
| future is suspended across some arbitrary bit of user code that isn't | ||
| guaranteed to ever finish. | ||
|
|
||
| [^illegitimate]: _Illegitimate_ ways to interfere with the runtime include | ||
| synchronous blocking in `poll`, calling unsafe functions like | ||
| [`pthread_cancel`], or just corrupting memory. That said, the [userspace | ||
| executors section][userspace_executors] does include a corner case where | ||
| safe and reasonable-looking user code can pause a whole single-threaded | ||
| `Runtime`. That might be worth preventing somehow, or it might be | ||
| considered low-level enough that documenting it is sufficient. | ||
|
|
||
| A fully formal definition of "promptly" will probably end up with somewhat | ||
| unsatisfying wording like "a finite period of time". Consider this excerpt from | ||
| [the C++ atomic memory model][finite]: |
There was a problem hiding this comment.
This is not unsatisfying, this is reflecting the way we can formally reason about liveness properties in a formal framework like LTL.
Saying "promptly" seems to imply some sort of quantitative property, like "within 1 second" or so. From what I understood about the bugs this is intended to fix, that is entirely unnecessary. The only guarantee we truly need is that it will eventually be called. ("eventually" is a formal operator in LTL and a standard ingredient in liveness properties.) How fast it is being called is a quality-of-implementation concern. But any system that arranges for the future to eventually be polled again will eventually make progress, and that is the key formal property that one typically reasons about in this context -- and that is supported by tools (e.g. TLA+), theory, and tons of research.
There was a problem hiding this comment.
There are definitely bugs that sit between the two.
Something like:
while let Some(item) = stream.next().await {
process(item).timeout_ms(1000).await;
}This doesn't deadlock because the loop body will never await longer than a second, but it's clearly an unintended consequence that the stream futures can sit around "un-polled" for that length of time.
It does seem a lot harder to set up a system when you can prove that this kind of bug does not exist though. Maybe you could treat each Future as a black box and then attempt to prove that progress will be made "eventually" without relying on details inside the future (such as it completing in 1000ms in this case).
There was a problem hiding this comment.
It does seem a lot harder to set up a system when you can prove that this kind of bug does not exist though.
Yeah this is more like how I'm thinking about it. This particular loop can't deadlock, but that's sensitive to what we put in the body, so I think it's fair to say that .next() as an individual component isn't living up to the intended contract. We can mix and match components like Timeout and Map and Then without ever causing a deadlock, but components like stream::Next and future::Shared are "deadlock prone". If we wanted to split hairs, we could say that they're not fundamentally broken and that they just need to document extensive requirements for their callers to use them correctly. (As Ralf said, none of this is UB after all.) But realistically, no one wants to ship a .next() method with a bunch of global preconditions and scary warnings attached to it and then blame users for inevitably using it wrong.
The examples I have in my head of someone "exploiting" the finite-period-of-time rule would be something like a runtime that deliberately sleeps for a couple seconds before polling tasks, or a future/stream adapter that spawns a thread to poll its children after a sleep. Obviously(?) we don't want runtimes or components to do silly things like that. But 1) it's not clear to me that there's a formal difference between doing unnecessary synchronous sleeps and e.g. a regular task queue that's just overloaded and bogged down. And 2) doing silly extra sleeps doesn't create non-local conflicts where we need a higher authority to tell us who's wrong; it's just a regular bug.
There was a problem hiding this comment.
I could definitely see a case where a runtime might decide to do short sleeps between polls in order to save power say.
There was a problem hiding this comment.
"eventually" is a formal operator in LTL and a standard ingredient in liveness properties
We could call this part of the contract the "poll eventually rule" instead of the "poll promptly rule"? It might be a tradeoff between clearly invoking the formal theory, vs describing what implementers should actually do 99% of the time.
There was a problem hiding this comment.
I don't think that's the right contract. MaybeNever is allowed to never call the future again
As @oconnor663 said, the implementation always polls the inner future to completion, it just makes whether that completion reaches the caller non-deterministic, so it's definitely possible to satisfy the contract. (Polling the future != the future returning Ready)
There was a problem hiding this comment.
Okay, I see now. You basically want to say that I cannot rely on any other future to ever be finished when arguing that I will invoke poll again.
I don't like that. Reasoning based on "that other code I wrote terminates hence this loop will terminate" is pretty standard liveness reasoning. I don't see a good reason to forbid it. I think this just makes the contract harder to understand and harder to reason about without sufficient benefit.
It's also oddly special-cased to wrap all futures in MaybeNever. If for some reason I implement timeout_ms with my own future-like type that's based on my own Poll-like type (and a custom executor that supports them) then obviously it wouldn't get wrapped and hence the new clause would change nothing. I don't see a reason why one would do that, but I also don't see a reason why it should make any difference for whether a given future satisfies the liveness requirements.
There was a problem hiding this comment.
Okay, I see now. You basically want to say that I cannot rely on any other future to ever be finished when arguing that I will invoke poll again.
Yes. I think this aligns well with the goal of allowing futures to make independent progress / preventing deadlocks or unwanted pauses.
Reasoning based on "that other code I wrote terminates hence this loop will terminate" is pretty standard liveness reasoning.
The opposite is also pretty standard to reason about with concepts like wait-freedom, where we must be able to prove that any thread can make progress without any other thread progressing. I think there are some pretty strong parallels here actually.
I don't see a good reason to forbid it. I think this just makes the contract harder to understand and harder to reason about without sufficient benefit.
You may be right, but I don't think it's clear cut. This contract is still relatively easy to understand and excludes some very common code bug classes with futures that the simpler contract does not.
It's also oddly special-cased to wrap all futures in MaybeNever.
We are talking about the contract of the Future trait, so it makes sense that it would be special-cased to implementations of that trait, no? Obviously if you made your own future-like trait you could make up whatever contract you want for that trait? That's true even for the simpler contract: someone could make a SnoozableFuture trait where it's allowed to never be polled again.
There was a problem hiding this comment.
The opposite is also pretty standard to reason about with concepts like wait-freedom, where we must be able to prove that any thread can make progress without any other thread progressing. I think there are some pretty strong parallels here actually.
The sleep happens in our "thread" (future) though, so we're not relying on other threads.
This contract is still relatively easy to understand
I beg to differ. ;)
We are talking about the contract of the Future trait, so it makes sense that it would be special-cased to implementations of that trait, no?
No. This is about "sources of waiting that are allowed to occur until poll gets called". I don't see why "waiting for a Future to become Ready" should be special there. It's not about the Future the contract is attached to after all, it's about all the other futures elsewhere in the program. There are many sources of waiting and there's no reason to special-case futures.
Like, for instance this is still allowed under your contract:
while let Some(item) = stream.next().await {
std::thread::sleep_ms(100);
}That seems kind of silly, to allow this but disallow the equivalent that's entirely built on Futures. (That program is obviously a bad idea, but that's not the point. It upholds unbounded liveness, and I explained above why unbounded liveness is a very desirable property even though there are trivial examples like this where it seems to be utterly useless.)
That's also why the rest of this paragraph is missing the point; it's not about the future the contract is attached to. Obviously MyFuture can decide not to have a contract, but that's entirely separate from the question of whether Future is allowed to wait for MyFuture (if we can prove that a particular MyFuture will definitely eventually be ready). You are essentially saying that Future must eventually be polled again and as caller I can rely on everything I know about the program except that calls to Future:poll are altered in a specific way (non-deterministically replacing Ready by Pending). But no other function call is altered even though obviously any other function could be doing literally the same thing! That has no parallel with anything I've ever seen, causes a lot of complications and problems for everyone who wants to reason about this systematically and formally, and doesn't actually solve any problem.
The more I think about this, the stronger I am opposed. We shouldn't unilaterally alter the user's program, not even for the purpose of hypothetical executions that we only use to define a liveness contract. Let the user reason about the code they actually wrote.
There was a problem hiding this comment.
The analogy with wait-freedom also does not hold up.
Wait-freedom is the strongest non-blocking progress guarantee for concurrent data structures,
ensuring that every operation completes in a finite number of steps regardless of interference from
other threads.
(source)
This is true for the original example as well. Your description as "we must be able to prove that any thread can make progress without any other thread progressing" is an inaccurate summary. It's about interference from other thread preventing progress on the current thread, e.g. by making CAS loops fail every single time. No amount of interference can prevent the sleep from completing, which means that the loop that sleeps is indeed wait-free.
Agreed. In the |
I am not sure we should allow arbitrary safe misbehavior. Is there a reason to allow it? |
|
Ah sorry, I meant to emphasize the "safe code" part. It was confusing of me to quote the part about panicking when what we're talking about here is the consequences of not calling That said, the current
|
|
Yes, calling But failing to call |
There was a problem hiding this comment.
Review Summary
Jack and I have been talking about this topic offline for the past couple of months, and I’m a big fan of more clearly stipulating the liveness rules for async Rust. This RFC has taken a lot of work, and a big thanks to Jack for driving progress on this!
There are however still some points we disagree on. I think the biggest one is on what "snoozing" actually means and how we should handle it. I believe that the proposed rules as written here are too complicated, and so I want to propose something simpler:
"When a future is polled, it must also poll all child futures that have had their waker called."
Jack defines a "snoozed future" as: "A future which is ready to make progress, but is not getting polled". Solutions that include terms like "promptly" or are based on eventual progress rely on accepting some amount of snoozing. Instead I'd like the Future trait to disallow snoozing entirely, which is both easier to reason about and provides a stronger guarantee.
Recommended Changes
Here is the full list of changes I’m recommending as part of this review, in descending order of importance:
- 🔴 Define “snoozed future”. The central term of this RFC is never explicitly defined by this RFC.
- 🔴 Make the polling guarantee more precise than “promptly”. We should be able to make stronger guarantees by using simpler rules which apply recursively.
- 🔴 Separate execution budgets from skipped polling. Budgets implemented by leaf futures constrain the callees, while the proposed contract constrains their callers.
- 🟡 Distinguish structured from unstructured concurrency. Problems which apply to unstructured operations like
select!do not apply to structured operations liketry_join. - 🔵 Explain alternatives to snoozing. We show what not to do in detail, but provide no guidance on what users are expected to do instead.
- 🔵 Clarify which stream abstractions are affected. The problems with “concurrent streams” do not apply to the
ConcurrentStreamtrait. - 🔵 Prior art for first-class concurrent execution. Swift’s
async letprovides prior art for a language-level concurrency feature.
| Clarify and document the contract requirements of [`Future::poll`], in | ||
| particular that a future should be polled or dropped promptly when it requests | ||
| a wakeup. In other words, we can cancel a future at any time, but we should | ||
| never "pause" or ["snooze"][snooze] a future. |
There was a problem hiding this comment.
Given how important this term is to the RFC, this RFC should probably explicitly define the term within the RFC. Right now I don't believe it does.
There was a problem hiding this comment.
This was a deliberate choice, whether or not it's the right one. I think snoozing/snoozed futures is an intuitive way to talk about the problem, especially for folks who've run into one of these deadlocks and are trying to understand what's going on. But I'm not sure it's the core concept we want to formalize. @RalfJung is the resident expert on liveness theory, and I don't know the first thing about it, so I'm probably going to make a mess of this, but in my head it seems tricky to draw a formal distinction between these three things:
- A basic snoozing deadlock, like one of the
select!loops we're talking about. - A future that has requested a wakeup, but whose task hasn't been polled yet, just because the task list is long and the executor is going as fast as it can.
- A future that has requested a wakeup, and whose task has been polled and in fact already returned from
poll, because that task previously handed this future off to some worker thread. The task notified the worker about this wakeup before returning frompoll, but that notification is still in flight.
We intuitively understand that the last two examples aren't bugs, they're just different ways that computers take non-zero time to do things. Those two futures are "going to be" polled in some reliable sense, so intuitively they're not "snoozed". But what exactly is the rule?
| often found at the scene of these crimes. Should we get our warning in `main` | ||
| by deprecating `select!`? In fact that's both too broad and also insufficient. | ||
| We can produce similar deadlocks with any form of cancellation. Here's [a | ||
| version using `timeout`][timeout_deadlock]. Here's [another version using | ||
| `try_join`][try_join_deadlock]. |
There was a problem hiding this comment.
Just because it's possible to make a mistake with structured concurrency operators such as Future::timeout and Future::try_join doesn't mean you're as likely to make a mistake with those operations as you are with unstructured operations like select!. In resilience engineering, this is known the difference between Safety-I and Safety-II. In the timeout playground you pin a future on one line, and then call it by-reference in the next line:
let fut1 = pin!(foo());
_ = timeout(Duration::from_millis(5), fut1).await;However, there is not really any need to pin that future on the first line in the first place. This might be common when using loop { select! { ... } }, but when using structured concurrency operations, it’s the norm to pass futures by-value everywhere. And so if we do that in this example, we no longer run into this bug (playground):
let fut1 = foo(); // <- no longer pinned in-place here
_ = timeout(Duration::from_millis(5), fut1).await;Even if we were to use Box::pin this isn't a problem since Box essentially encodes &own semantics rather than &mut semantics (playground):
let fut1 = Box::pin(foo()); // <- `Box::pin`
_ = timeout(Duration::from_millis(5), fut1).await;That doesn’t mean that the point you’re arguing in this RFC isn’t useful or good, but I don’t agree with your claim that migrating off of unstructured concurrency operations would make no difference.
There was a problem hiding this comment.
You've mentioned this structured vs unstructured concept before (you also call it "Tree-Structured Concurrency" on your blog yes?), but it really does not seem to me like a difference between what select! does and what these other primitives do. If I understand you correctly, this way of doing things is "structured":
use futures_concurrency::future::Race;
(foo(), bar()).race().await;And this way is "unstructured":
select! {
_ = foo() => {}
_ = bar() => {}
}But those two examples are totally equivalent. I agree with you that it's more common for programmers to do fishy reference-y stuff like this with select!:
let foo_ref = pin!(foo());
select! {
_ = foo_ref => {}
_ = bar() => {}
}But that's also totally equivalent to doing the same thing with race:
let foo_ref = pin!(foo());
(foo_ref, bar()).race().await;I think the fundamental distinction here is between "driving futures by value" (good, reliable, tends to abide by the strict contract) and "driving futures by reference" (bad, deadlock-prone, violates the contract when combined with any form of cancellation). But when we put it that way, it seems clear to me that select! vs race vs timeout distinction is a red herring. They're all fine when working by value, and not fine when working by reference.
However, there is not really any need to pin that future on the first line in the first place. This might be common when using
loop { select! { ... } }, but when using structured concurrency operations, it’s the norm to pass futures by-value everywhere.
My first example in the Motivation section includes the sort of loop that leads to pinning, but I've left it out of some of the other variants of that example for brevity. That might've been confusing. As I think you know, we can write the exact same loop using timeout if we want to. In fact, some of my earlier drafts did it this way, and I went back and forth a couple times.
// While `bar` is running, call `baz` every 5 ms.
let mut bar_future = pin!(bar());
loop {
_ = timeout(Duration::from_millis(5), &mut bar_future).await;
baz().await; // Deadlock!
}As you've mentioned, programmers seem to choose to do things like this with select! more often than with timeout. I wonder if that's partly because the timeout example here feels wrong in a way that the select! version doesn't? I'm not sure.
Even if we were to use
Box::pinthis isn't a problem
I'm can't tell whether this is already clear, but all the crimes we can commit with pin! we can also commit with Box::pin, because of the blanket Future impl on Pin<&mut _> combined with the other blanket Future impl on unpinned &mut _. For example:
let mut fut1 = Box::pin(foo());
_ = timeout(Duration::from_millis(5), &mut fut1).await;(Alternatively we could call .as_mut() on it.)
There was a problem hiding this comment.
What I'm trying to say is that the only time I've ever seen futures explicitly being driven by reference was when used with select! {}. That is because select! {} needs to be able to poll by reference for many of its uses.
It technically being possible to pass futures by-ref to other APIs that don't encourage doing this doesn't mean that those APIs create the same problems select! {} does. To me that's a distinction well worth making.
There was a problem hiding this comment.
It sounds like we mostly agree here, and we're just describing the same situation in different ways. But I still want to be careful about calling it "technically" possible, as if to say that real projects don't do this sort of thing in production. Here's Buck2 [1] and Codex [2] doing it with timeout. Here's OpenVMM [3] doing it with try_join. Here's Quic-RPC [4] (yooo @rklaehn @dignifiedquire how's it going) doing it with race. Here's Boa [5] doing it with smol's or method.
|
|
||
| The `select!` deadlock [discussed above][motivation], and the `timeout` and | ||
| `try_join` versions from the same section, are all examples of what we might | ||
| call "cancellation by reference". We can cause a deadlock anywhere cancellation |
There was a problem hiding this comment.
As I showed in my other comment, structured operations like timeout and try_join rarely (if ever) exercise cancellation-by-reference, which is by design. Listing them alongside an unstructured operation like select! here feels wrong.
| ### Pausing things is useful, and it would've been nice to allow it. | ||
|
|
There was a problem hiding this comment.
I feel like this section really ought to point out that we can safely pause futures, despite the limitations that the additional guarantees we're imposing in this RFC. What we're disallowing is not polling futures when they're ready. What we can still do is wait for two futures concurrently, where one takes longer than the other.
The most straight-forward implementation of this is probably something like this:
use futures_concurrency::prelude::*;
use async_channel::bounded;
let a = async { "hello" };
let (sender, receiver) = bounded::<()>(1);
// pretend we hand the `sender` off to somewhere here
// `a` resolves immediately. But `receiver` will
// not resolve until `sender` fires, safely
// pausing `a` from yielding its output and the
// caller from progressing.
let (a, _) = (a, receiver).join().await;This however doesn’t block the underlying computation, it only blocks the result of the computation being processed. If you want to be able to block computations, we can encode those as asynchronous streams which make incremental progress, and use operations like Stream::zip to provide granular control on every tick.
| The usual expected behavior is that an implementation of `poll` should call | ||
| `poll` on all of its child futures (if any) before returning. However, there | ||
| are other valid ways to do things. Some async runtimes may [track an execution | ||
| "budget"][budget], such that some of their futures return `Pending` earlier |
There was a problem hiding this comment.
I don't believe execution budgets as described in the Tokio post have any bearing on the polling behavior. Execution budgets are a property of the callees (leaf futures being polled), not of the callers (intermediate futures calling other futures). But the proposed futures contract is however a property of the callers.
To give an example, imagine we’re concurrently awaiting four file operations:
let a = File::open("a.md");
let b = File::open("b.md");
let c = File::open("c.md");
let d = File::open("d.md");
let [a, b, c, d] = [a, b, c, d].try_join().await?;All four of these operations happen on the same task, and so are subject to the execution budget. That means that even though all four File::open futures might be ready, the budget might run out halfway through, and so on wake we end up with [Ready, Ready, Pending, Pending]. At no point did we skip polling any of the futures, resulting in something like [Ready, Ready, <snooze>, <snooze>]. We poll all futures, and if some resources fire a waker and return Poll::pending that seems fine.
There was a problem hiding this comment.
I looked further into this and I did find one API in Tokio which relies on being able to snooze futures for a little bit: tokio::task::coop::cooperative. The way it works is that based on the value of some task-local counter, it will snooze some T: Future and schedule it to be woken again on some future tick. This is what the inner future looks like in pseudo-code:
match EXECUTION_BUDGET.get().read() {
// Task budget depleted, schedule wake and return `Pending`.
0 => {
cx.waker().wake_by_ref();
Poll::Pending
}
// Task budget available, decrement budget and poll inner future.
1.. => {
*EXECUTION_BUDGET.get().write() -= 1;
self.0.poll(cx)
}
}Disallowing snoozes entirely here is relatively straight forward however. Instead of disallowing forward progress of the underlying computation, we can delay making forward progress on future completion by caching the Future::output if necessary. This is more or less the same as how a budget-aware async TCP connection would behave: progress happens concurrently, we do not halt that progress, but we can delay yielding the output.
// Where `self.output` is of type `Option<Future::Output>`
match (EXECUTION_BUDGET.get().read(), self.output) {
// Budget available, no cached output. Poll and return as normal.
(1.., None) => {
*EXECUTION_BUDGET.get().write() -= 1;
self.0.poll(cx)
}
// Budget available, has cached output. Return cached output.
(1.., Some(output)) => {
*EXECUTION_BUDGET.get().write() -= 1;
Poll::Ready(self.output.take())
}
// Budget unavailable, no cached output. Poll and cache output.
(0, None) => {
let output = ready!(self.0.poll(cx));
self.output = Some(output);
Poll::Pending
}
// Budget unavailable, has cached output. Schedule wake, return pending.
(0, Some(output)) => {
cx.waker().wake_by_ref();
Poll::Pending
}
}There was a problem hiding this comment.
You're totally right that I misunderstood how Tokio does it. Now that I go looking, a better example of a future that can yield control before polling all its children is FuturesUnordered: https://github.com/rust-lang/futures-rs/blob/0.3.34/futures-util/src/stream/futures_unordered/mod.rs#L551-L559
| A fully formal definition of "promptly" will probably end up with somewhat | ||
| unsatisfying wording like "a finite period of time". Consider this excerpt from | ||
| [the C++ atomic memory model][finite]: |
There was a problem hiding this comment.
Like what Ralf is saying about this in the other thread, the usage of "promptly" does not seem quite right with me either. But I think we might actually be able to go further than just guaranteeing a future will eventually wake up, by applying a rule which recursively applies to all futures:
"When a future is polled, it must also poll all child futures that have had their waker called."
I think this one rule is a lot easier to intuit for people, while not placing any restrictions on schedulers. If you're manually writing a future, if it holds any sub-futures (typically some: T: Future), then it must always poll them if they had their waker called. That rule should be easy enough to follow and prevents snoozes entirely.
There was a problem hiding this comment.
This rephrasing seems more restrictive while introducing its own questions:
Questions it raises:
- What exactly counts as a "child future"
- What is the synchronisation point between the future being called and the waker being called (these often happen on different threads)
Restrictions it imposes:
- Futures can no longer be polled asynchronously. A
FuturesUnordered-style combinator may wish to "yield" to the executor between each poll to a child future to avoid starving other tasks running on the executor (triggering its own waker to ensure that other ready child futures are still polled promptly)
The current definition is IMO simpler to understand even if it is harder to formalise.
There was a problem hiding this comment.
A
FuturesUnordered-style combinator may wish to "yield" to the executor between each poll to a child future to avoid starving other tasks running on the executor […]
That is actually a good example of snoozed futures causing problems, and what a stricter future contract would want to prohibit. The kind of scheduling you're describing is what leads to similar kinds of mystery timeouts as described by the Barbara Battles Buffered Streams scenario.
There was a problem hiding this comment.
What exactly counts as a "child future"
The futures trait would generally benefit from a better description of all valid future states. But by "child future" I mean any live future currently owned by some other future (the "parent future"). By. "live future" I mean any future which has been polled at least once and has not yet returned Poll::Ready, panicked, or otherwise been poisoned.
There was a problem hiding this comment.
"owned" is purely a type system concept, it does not exist at runtime, so I am not sure we can make it a basis for a definition of runtime behavior.
For example, if a future is stored in an Arc<Mutex<_>> that many parties hold a reference to, who is the "owner" of that future?
There was a problem hiding this comment.
That is actually a good example of snoozed futures causing problems, and what a stricter future contract would want to prohibit. The kind of scheduling you're describing is what leads to similar kinds of mystery timeouts as described by the Barbara Battles Buffered Streams scenario.
I don't think that's correct. The FuturesUnordered behavior that @Diggsey is referring to is yielding to the executor early when one of the child futures has already requested a wakeup (that is, not just requested in the past, but requesting again during their own poll). In that case FuturesUnordered is going to get polled again, so this by itself can't cause a deadlock.
There was a problem hiding this comment.
"owned" is purely a type system concept, it does not exist at runtime, so I am not sure we can make it a basis for a definition of runtime behavior.
Yeah this is a problem with the current wording. In my head it's like "if you call poll and give it a Waker, then you have to respect that Waker". But "you" here feels like it has more to do with privacy boundaries than with e.g. tasks or threads.
Could we get away with saying something like: This is the documented contract of poll. If "you" call poll, you either have to guarantee yourself that the contract is followed, or you have to impose requirements on your own caller such that the contract will be followed if the caller satisfies your requirements.
There was a problem hiding this comment.
For example, if a future is stored in an
Arc<Mutex<_>>that many parties hold a reference to, who is the "owner" of that future?
@RalfJung I think you might be over-indexing on the phrasing I used. The point I'm trying to make is that we should define clearer terms for common future uses and states so that people can better reason about them. How exactly we phrase things we can discuss and iterate on.
For example, we could caveat what I wrote with something like: “We assume you’re not forcing a future to have multiple concurrent owners by sticking it in a shared lock, because that would be silly.”
I don't think that's correct [...] this by itself can't cause a deadlock.
@oconnor663 The central problem in the "Barbara battles buffered streams" scenario isn't a deadlock, it's a timeout. The only way to prevent timeouts like this from occurring is to disallow snoozing altogether.
There was a problem hiding this comment.
That is actually a good example of snoozed futures causing problems, and what a stricter future contract would want to prohibit. The kind of scheduling you're describing is what leads to similar kinds of mystery timeouts as described by the Barbara Battles Buffered Streams scenario.
No it's not - that's a completely different situation. In that situation poll is not being called promptly. In my situation it is being called promptly, since the parent future is re-waking itself before returning to the executor.
| x += 1; | ||
| } | ||
| assert_eq!(x, 2); | ||
| ``` |
There was a problem hiding this comment.
Nit: Swift uses the async let notation for concurrently awaiting tasks. In my post Automatic Interleaving of High-Level Concurrent Operations I show how we might be able to adapt that same system to Rust using .co_await to introduce the notion of a concurrent await:
async fn make_dinner() -> SomeResult<Meal> {
let veggies = chop_vegetables().co_await?;
let tofu = marinate_tofu().co_await?;
let oven = preheat_oven(350).co_await;
let dish = Dish(&[veggies, tofu]).co_await;
oven.cook(dish, Duration::from_mins(3 * 60)).await
}What I did not cover in that post is that, as you point out, by introducing a first-class notation for concurrent operations in Rust, we get to define how that should interact with the borrow checker. That was the main reason we introduced async/.await to Rust at the time as well, despite some in the community arguing that Future::map would be sufficient.
If we adapted your example to use concurrent .await, that would look something like this:
let mut x = 0;
async {
sleep(Duration::from_secs(1)).await;
x += 1;
}.co_await;
async {
sleep(Duration::from_secs(1)).await;
x += 1;
}.co_await;
assert_eq!(x, 2);As you point out, the exact rules for what is/isn't allowed here are pretty nuanced. But I mostly wanted to add to this that I like the idea of having a first-class notation for concurrent execution, there is prior art for that in Swift, and we should be able to extend that with your proposed semantics if we wanted to.
View all comments
Require a future's owner to poll it when it requests a wakeup, to avoid snoozing bugs like "Futurelock". This RFC is a docs-only change, but this will have far-reaching consequences for a lot of common async patterns.
RFC #4020 is being submitted at the same time. It adds
poll_progresstoAsyncIterator, in part to satisfy the strictFuturecontract introduced in this RFC.Important
Since RFCs involve many conversations at once that can be difficult to follow, please use review comment threads on the text changes instead of direct comments on the RFC.
If you don't have a particular section of the RFC to comment on, you can click on the "Comment on this file" button on the top-right corner of the diff, to the right of the "Viewed" checkbox. This will create a separate thread even if others have commented on the file too.
Rendered