Swatinem Blog Resume

Non-Lazy Futures Considered Harmful

— 4 min

Now 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 structs 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: