Non-Lazy Futures Considered Harmful
— 4 minNow that I got your attention with the clickbait title, let me explain what I mean by it. Current Rust code could be broken in very subtle ways because of some assumptions we have about async Rust code that might not always be true.
This story starts with
a bug I recently fixed in sentry-rust
(which manifested itself as a memory leak), and itself highlights both sides of
the problem.
The root cause of the problem is not specific to the Sentry SDK however, and it
can very easily happen in other cases as well. I will use examples from the
tracing
ecosystem down below, as that might have a wider user base than Sentry.
The problem boils down to the very subtle difference between async fn X()
and
fn X() -> impl Future
, and the fact that the second is not guaranteed to be
fully lazy.
An async fn
is by definition lazy. Calling it via async_fn()
only captures
the arguments into an anonymous Future
type, but is otherwise guaranteed to
not execute the body of the function yet, but rather later on poll
.
This is not the case for a function that returns an impl Future
, and by
extension for trait functions that return (generic or named) futures.
These functions can have a varying amount of code that runs at call time,
vs poll time.
fn fully_lazy() -> impl Future<Output = ()> {
async move {
println!("print happens on *poll*");
}
}
fn not_lazy() -> impl Future<Output = ()> {
println!("print happens on *call*");
std::future::ready(())
}
So if possible, move all of the code into the async block. But that might not always be possible.
# Aside: Named Types
We do have the problem right now that async fn
returns an anonymous type.
Returning impl Future
has the same problem. You cannot name the type in
stable Rust today. And named types are required when you want to put a future
on a struct
or into a collection. It is also considered good practice in
general to name every public API type of a crate, for that reason.
The same principle is also the reason why every Iterator
combinator returns
its own named type. And it is the reason why there is
tracing::instrument::Instrumented
and sentry::SentryFuture
.
Nightly Rust offers the
type_alias_impl_trait
feature,
which allows giving names to otherwise anonymous futures. Being a nightly feature, it is not yet available in stable Rust unfortunately.
The next best thing we can use on stable Rust is Pin<Box<dyn Future>>
, like so:
fn dyn_future() -> Pin<Box<dyn Future<Output = ()>>> {
Box::pin(async move {
println!("print happens on *poll*");
})
}
That is also the desugaring that the async-trait
crate does (well
almost). The
resulting type has a name, and we can put it into struct
s and collections.
However it comes with the disadvantage of a heap allocation and dynamic dispatch.
And it is not available for #![no_std]
builds either.
So the broader community seems to have settled on creating their own Future
types, which often wrap an inner future via generics.
That on its own comes with a lot of inconvenience, as manually implementing
Future::poll
is a nightmare, and often requires manual unsafe
code, or
pulling in an external dependency in the form of pin-project.
For that reason, I have resorted to the following pattern, and I bet a lot of other people do so as well:
fn returns_named_future() -> NamedFuture {
// do as much logic as you possibly without having to use `await`
// return the resulting future, which does the rest inside `poll`
NamedFuture
}
#
Problematic tracing
example
To further illustrate the problem, let me demonstrate both problems with a full
practical tracing
example, which follows the guidelines as presented in
the tracing::Span
docs:
use std::future::Future;
use tracing::Instrument;
fn fully_lazy() -> impl Future<Output = ()> {
async move {
tracing::info!("log happens on *poll*");
}
}
fn not_lazy() -> impl Future<Output = ()> {
tracing::info!("log happens on *call*");
std::future::ready(())
}
fn broken_parent_lazy() -> impl Future<Output = ()> {
let span = tracing::info_span!("broken_parent");
fully_lazy().instrument(span)
}
fn broken_parent_not_lazy() -> impl Future<Output = ()> {
let span = tracing::info_span!("broken_parent");
not_lazy().instrument(span)
}
fn correct_parent_lazy() -> impl Future<Output = ()> {
let span = tracing::info_span!("correct_parent");
span.in_scope(|| fully_lazy()).instrument(span)
}
fn correct_parent_not_lazy() -> impl Future<Output = ()> {
let span = tracing::info_span!("correct_parent");
span.in_scope(|| not_lazy()).instrument(span)
}
#[tokio::main]
async fn main() {
tracing_subscriber::fmt::init();
broken_parent_lazy().await;
broken_parent_not_lazy().await;
correct_parent_lazy().await;
correct_parent_not_lazy().await;
}
Running this example outputs the following:
2022-01-26T14:48:11.115115Z INFO broken_parent: lazy_futures: log happens on *poll*
2022-01-26T14:48:11.115511Z INFO lazy_futures: log happens on *call*
2022-01-26T14:48:11.115732Z INFO correct_parent: lazy_futures: log happens on *poll*
2022-01-26T14:48:11.116020Z INFO correct_parent: lazy_futures: log happens on *call*
As we can see, the second log line (coming from broken_parent_not_lazy().await
)
is not running in correct span as intended. The other examples work as intended,
as either the caller handles the unlikely case of code being executed in call,
or rather the callee is more correct by not executing any code in call.
# Conclusion
I have two recommendations here, covering both sides of the coin:
-
Do not make assumptions about third-party code being lazy. Foreign code might be misbehaving in a way that it executes actual code in call vs poll.
-
Make all your code fully lazy. Do not do any computations in call vs poll. Foreign code may rely on futures code being fully lazy, and you might otherwise violate expectations.