rustc fails to prove Sendness for an async block where a !Send value is dropped before an .await point

[frank-king]

I tried this code: (playground)

#[tokio::main]
async fn main() {
    let (tx, rx) = tokio::sync::watch::channel(vec![1, 1, 4, 5, 1, 4]);
    let mut binding = rx.clone();
    tokio::spawn(async move {
        let value = binding.borrow();
        let len = value.len();
        drop(value);
        println!("len: {}", len);
        binding.changed().await
            .expect("watch error");
    }).await;
    println!("{:?}", rx.borrow());
}

I expected to see this happen: the code passes compilation.

Instead, this happened:

error: future cannot be sent between threads safely
   --> src/main.rs:5:5
    |
5   | /     tokio::spawn(async move {
6   | |         let value = binding.borrow();
7   | |         let len = value.len();
8   | |         drop(value);
...   |
11  | |             .expect("watch error");
12  | |     }).await;
    | |______^ future created by async block is not `Send`
    |
    = help: within `{async block@src/main.rs:5:18: 5:28}`, the trait `Send` is not implemented for `std::sync::RwLockReadGuard<'_, Vec<i32>>`, which is required by `{async block@src/main.rs:5:18: 5:28}: Send`
note: future is not `Send` as this value is used across an await
   --> src/main.rs:10:27
    |
6   |         let value = binding.borrow();
    |             ----- has type `tokio::sync::watch::Ref<'_, Vec<i32>>` which is not `Send`
...
10  |         binding.changed().await
    |                           ^^^^^ await occurs here, with `value` maybe used later
note: required by a bound in `tokio::spawn`
   --> /playground/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.38.0/src/task/spawn.rs:166:21
    |
164 |     pub fn spawn<F>(future: F) -> JoinHandle<F::Output>
    |            ----- required by a bound in this function
165 |     where
166 |         F: Future + Send + 'static,
    |                     ^^^^ required by this bound in `spawn`

error: future cannot be sent between threads safely
   --> src/main.rs:5:5
    |
5   | /     tokio::spawn(async move {
6   | |         let value = binding.borrow();
7   | |         let len = value.len();
8   | |         drop(value);
...   |
11  | |             .expect("watch error");
12  | |     }).await;
    | |______^ future created by async block is not `Send`
    |
    = help: within `{async block@src/main.rs:5:18: 5:28}`, the trait `Send` is not implemented for `*mut ()`, which is required by `{async block@src/main.rs:5:18: 5:28}: Send`
note: future is not `Send` as this value is used across an await
   --> src/main.rs:10:27
    |
6   |         let value = binding.borrow();
    |             ----- has type `tokio::sync::watch::Ref<'_, Vec<i32>>` which is not `Send`
...
10  |         binding.changed().await
    |                           ^^^^^ await occurs here, with `value` maybe used later
note: required by a bound in `tokio::spawn`
   --> /playground/.cargo/registry/src/index.crates.io-6f17d22bba15001f/tokio-1.38.0/src/task/spawn.rs:166:21
    |
164 |     pub fn spawn<F>(future: F) -> JoinHandle<F::Output>
    |            ----- required by a bound in this function
165 |     where
166 |         F: Future + Send + 'static,
    |                     ^^^^ required by this bound in `spawn`

However, this code works fine: (playground)

#[tokio::main]
async fn main() {
    let (tx, rx) = tokio::sync::watch::channel(vec![1, 1, 4, 5, 1, 4]);
    let mut binding = rx.clone();
    tokio::spawn(async move {
        let len = {
            let value = binding.borrow();
            value.len()
        };
        println!("len: {}", len);
        binding.changed().await
            .expect("watch error");
    }).await;
    println!("{:?}", rx.borrow());
}

Meta: nightly rust (1.81.0-nightly 2024-07-12) and stable rust (1.79.0) behave similarly.

I have constructed a minimum reproducible version:

#![allow(unused)]

struct NotSend(std::marker::PhantomData<*mut ()>);

impl NotSend {
    fn new() -> Self {
        Self(std::marker::PhantomData)
    }
}

unsafe impl Sync for NotSend {}

#[derive(Clone)]
struct WatchRecv;

impl WatchRecv {
    fn borrow(&self) -> Ref<'_> {
        Ref(self, NotSend::new())
    }
}

struct Ref<'a>(&'a WatchRecv, NotSend);

fn assert_send<F: std::future::Future + Send>(f: F) -> F { f }

impl Ref<'_> {
    fn len(&self) -> usize { 0 }
}

async fn another_future() {
    loop {}
}

async fn real_main() {
    let rx = WatchRecv;
    let mut binding = rx.clone();
    assert_send(async move {
        // /*
        // This doesn't works.
        let value = binding.borrow();
        let len = value.len();
        drop(value);
        println!("len: {len}");
        // */
        /*
        // This works.
        let len = {
            let value = binding.borrow();
            value.len()
        };
        */
        another_future().await;
    }).await;
    println!("{:?}", rx.borrow().len());
}

[frank-king]

I dag into rustc_trait_selection for a while but didn't find a clear answer. Is it because the trait obligation selection is based on lexical scopes instead of non-lexical lifetimes?

[compiler-errors]

This isn't anything to do with trait selection. I believe this has to do with the temporary that we create on the autoderef when calling the .len() method. Here's a minimal example:

async fn yield_point() {}

struct Lock;
impl Lock {
    fn borrow(&self) -> LockGuard<'_> { todo!() }
}

struct LockGuard<'a>(*mut (), &'a ());
impl std::ops::Deref for LockGuard<'_> {
    type Target = Inner;
    fn deref(&self) -> &Self::Target { todo!() }
}

struct Inner;
impl Inner {
    fn foo(&self) -> i32 { 0 }
}

fn main() {
    let lock = Lock;

    fn is_send(_: impl Send) {}
    is_send(async {
        let guard = lock.borrow();
        let _len = guard.foo(); // Comment this out and it works.
        drop(guard);
        yield_point().await;
    });
}

[compiler-errors]

Actually, here's a more minimal example:

#![feature(coroutines)]

struct S(*mut ());
impl S {
    fn do_something(&self) {}
}

fn main() {
    fn is_send(_: impl Send) {}
    is_send(#[coroutine] static || {
        // Make a new `S`.
        let s = S::new();
        // Borrow it by calling a method on it.
        s.do_something();
        // Drop `s` by moving it.
        drop(s);
        // Closure analysis keeps thinking that it's live since it has been
        //  borrowed and there's no `StorageDrop` on `s`'s local.
        yield;
    });
}

It seems to do with the fact that we don't consider the variable s to be dead even though it's unconditionally moved from.

[compiler-errors]

cc #112279 which fixes this

[yhx-12243]

#104883 (comment)

[traviscross]

@rustbot labels +AsyncAwait-Triaged

We discussed this in our meeting today. Looks like it's being fixed.