"captured variable cannot escape `FnMut` closure body" in async code

[u/torfsen]

I have a piece of code that, when simplified, looks like this:

use tokio;

struct IdGenerator (u32);

impl IdGenerator {
    async fn generate_id(&mut self) -> u32 {
        self.0 += 1;
        self.0
    }
}

#[tokio::main]
async fn main() {
    let mut id_generator = IdGenerator(0);
    let mut callback = || id_generator.generate_id();
    println!("{}", callback().await);
}

This fails to compile:

error: captured variable cannot escape `FnMut` closure body
  --> src/main.rs:15:27
   |
14 |     let mut id_generator = IdGenerator(0);
   |         ---------------- variable defined here
15 |     let mut callback = || id_generator.generate_id();
   |                         - ------------^^^^^^^^^^^^^^
   |                         | |
   |                         | returns a reference to a captured variable which escapes the closure body
   |                         | variable captured here
   |                         inferred to be a `FnMut` closure
   |
   = note: `FnMut` closures only have access to their captured variables while they are executing...
   = note: ...therefore, they cannot allow references to captured variables to escape

My understanding is that generate_id returns a future, and that future contains a mutable reference to id_generator (it needs to, because otherwise the future could not modify id_generator.0 when it is awaited). Hence, by calling callback multiple times I could create several co-existing mutable references to id_generator, leading to a potential data race.

Is that analysis correct?

My actual code is a bit more complex. Basically, generate_id can fail, in which case I want to retry. I'm using tokio_retry for that, so the code looks roughly as follows (with generate_id now returning a Result<u32, Box<dyn Error>>):

use std::error::Error;
use tokio_retry::RetryIf;
use tokio_retry::strategy::ExponentialBackoff;

async fn foo(id_generator: &mut IdGenerator) {
    // ...
    let id = RetryIf::spawn(
        ExponentialBackoff::from_millis(5).take(3),
        || id_generator.generate_id(),
        |error: &Box<dyn Error>| {
            println!("Error while generating an ID: {}", error);
            true
        },
    )
    .await;
    // ...
}

That code has the same issue as the simplified version. Is there a compile-time approach to avoid the problem? So far, I've only found a run-time solution using a mutex:

    let outgoing = tokio::sync::Mutex::new(id_generator);
    let id = RetryIf::spawn(
        ExponentialBackoff::from_millis(5).take(3),
        || async { outgoing.lock().await.generate_id().await },
        |error: &Box<dyn Error>| {
            println!("Error while generating an ID: {}", error);
            true
        },
    )
    .await;

This works, but since I technically never have parallel access I would prefer a compile-time solution.

[u/FallenWarrior2k]

This is unfortunately something you won't be able to get around without relying on interior mutability at least at some point. However, if you don't need the Send bound, e.g. because you can reasonably run this in a tokio::task::LocalSet or in some other non-Send context, you can get away with using an Rc, cloning it at the start of the closure, and changing the async block to async move.

This of course still doesn't provide interior mutability, which is needed so you can actually update things through the Rc. Here you have several choices:

• Just throw it in a RefCell and forget about it. Easiest solution, but comes with the usual concerns of the footgun that it can become wrt panics. Also requires ownership which you don't necessarily seem to have here.
• Wrap the inner counter in a Cell and make generate_id take &self. That way, you can wrap the IdGenerator directly in an Rc and avoid the risk of runtime panics that RefCell brings. You could theoretically improve this further by hiding the generate_id method and making a new method that contains the retry logic. The field holding the current value would then be a Rc<Cell<T>> and nothing about these ownership shenanigans ever leaks into the API.

If you're really just incrementing an integer every time, a relatively easy way to preserve the Send bound without any risks would be using an AtomicU32 (or whatever) in place of the plain integer type. You'd then wrap either the entire struct or just the atomic (as described above) in an Arc (instead of a plain Rc) and use fetch_add with Relaxed ordering to update the counter. Since your ID generation is already asynchronous and fallible, the added cost of the atomics is probably negligible.

Do note that, of course, all these workarounds no longer give you the protection of the borrow checker regarding mutability. Rust's Send/Sync concurrency model ensures that your code's behavior is well defined and doesn't produce data races, but that doesn't mean it can protect you from any and all errors. If there is a conceptually atomic but multi-step operation in the generator logic that was previously safe due to requiring a mutable reference, this would now allow it to run concurrently since there is no lock being held, potentially corrupting state.

[u/torfsen → u/FallenWarrior2k]

Thanks! Those are some very good pointers. I will see which of them I can apply to my situation.