rust-lang / rust

Empowering everyone to build reliable and efficient software.
https://www.rust-lang.org
Other
97.33k stars 12.59k forks source link

Overly conservative async capture analysis when values are borrowed #129325

Open nikomatsakis opened 1 month ago

nikomatsakis commented 1 month ago

There's probably a bug filed already but this code does not compile, though it really ought to:

use std::sync::Mutex;

async fn foo(m: &Mutex<u32>) {
    let lock = m.lock().unwrap();
    if condition(&lock) {
        drop(lock);
        return bar().await;
    }

    drop(lock);
    return bar().await;
}

async fn bar() { }

fn is_send<T: Send>(t: T) { }

fn condition(x: &u32) -> bool {
    false
}

fn main() {
    let m = Mutex::new(22);
    is_send(foo(&m));
}

I believe the problem is specific to the &lock, which causes the capture analysis to get nervous -- even though lock is dropped. This is distilled from real-world code within Amazon.

Error you get today:

error: future cannot be sent between threads safely
  --> src/main.rs:26:13
   |
26 |     is_send(foo(&m));
   |             ^^^^^^^ future returned by `foo` is not `Send`
   |
   = help: within `impl Future<Output = ()>`, the trait `Send` is not implemented for `MutexGuard<'_, u32>`, which is required by `impl Future<Output = ()>: Send`
note: future is not `Send` as this value is used across an await
  --> src/main.rs:9:22
   |
6  |     let lock = m.lock().unwrap();
   |         ---- has type `MutexGuard<'_, u32>` which is not `Send`
...
9  |         return bar().await;
   |                      ^^^^^ await occurs here, with `lock` maybe used later
note: required by a bound in `is_send`
  --> src/main.rs:18:15
   |
18 | fn is_send<T: Send>(t: T) { }
   |               ^^^^ required by this bound in `is_send`

I'm nominated for async just to get some eyes on this. I'd be interested to discuss fixes, I have a few thoughts, though I'd have to look at the code too.

nikomatsakis commented 1 month ago

This is a variant of sorts of https://github.com/rust-lang/rust/issues/57017

compiler-errors commented 1 month ago

Possibly fixed by #128846, which I would like to get landed eventually.

compiler-errors commented 1 month ago

Yes, it is fixed by that PR.

nikomatsakis commented 1 month ago

Ooh, nice