`Sync` bound in async code is overly restrictive

Example code:

use std::cell::Cell;

async fn foo<T>(x: &T) {}

async fn bar() {
    let x = Cell::new(1);
    foo(&x).await;
}

fn assert_send<T: Send>(_: T) {}

fn main() {
    assert_send(bar());
}

This will fail because x does not implement Sync, which in turn means that the future returned by bar() does not implement Send (because x is borrowed across a yield point).

However, this is overly restrictive: because bar() owns x, and polling a future requires a mutable reference, we know that x will never be accessed concurrently by multiple threads.

It also puts async code into a weird position where functions taking a mutable reference are actually easier to call / callable in more places:

use std::cell::Cell;

async fn foo<T>(x: &mut T) {}

async fn bar() {
    let mut x = Cell::new(1);
    foo(&mut x).await;
}

fn assert_send<T: Send>(_: T) {}

fn main() {
    assert_send(bar());
}

The question is: how can we allow the code in the first example without breaking the safety invariants?

That exclusivity is only true if you can prove that a reference didn't leak anywhere else. Hypothetically, what if foo spawned a scoped thread of some sort that independently uses that reference?

Firstly, these "rules" for when this is OK are exactly what I'm trying to nail down by asking this question, so thanks!

However, I think your specific example doesn't work: if foo spawned a thread and attempted to move the reference into it, then foo would require a Sync bound on the type anyway.

1 Like

Fair point, but maybe there are other ways to escape, like a scoped thread-local?

Generally speaking, it seems quite hard to prove that a particular &T would always refer to an owned T in the same (or nested) future, but that's just my rough intuition.

The example is a bit more complicated than necessary. An easier one is

use std::cell::Cell;

async fn foo() {
    let x = &Cell::new(1);
    async{}.await;
    drop(x);
}

fn assert_send<T: Send>(_: T) {}

fn main() {
    assert_send(foo());
}

The point being: If any kind of analysis allowing holding references over an await was to be added, this trivial case, without any calls to other async functions, would be allowed for sure.

1 Like

For non-'static references it seems to me that any attempt to "escape" the reference this way would be visible in the signature of the root Future type, because you'd need a lifetime bound to guarantee that the value you're storing into the thread-local would live long enough.

For 'static references it's a bit trickier. On the other hand, it's extremely rare for 'static data to be !Sync, so even if this could only be made to work for non-'static data it would be very useful.

1 Like

If instead of Cell you use an Rc, it's easy to create unsafety by simply storing a clone of it in a thread-local variable, with then leads to a data race on the reference count.

Similarly with Box::leak on a Cell.

With a non-'static non-Rc type it seems a bit harder, but it looks like a generally safe API that creates UB in this case can be created: specifically a TLS API storing Option<T> that are forgotten instead of freed where the lifetime-bound key can be sent between threads and thus sent back to the original thread after the future is moved to a different thread.

Seems hard to find a simple way to allow this, other than using some sort of unsafe assertion.

2 Likes

Without inspecting foo in some way, bar could not be assumed to return a future that’s Send.

From its signature (including the non-explicit information) foo is generic over T and the future it returns is only Send if T is Sync. One can easily do bad stuff inside such a future leading to unsound behavior if bar is sent between threads. For example, foo could use specialization to do the extra nasty stuff only when T is a Cell. Here’s an example I came up with.

1 Like

If instead of Cell you use an Rc, it's easy to create unsafety by simply storing a clone of it in a thread-local variable, with then leads to a data race on the reference count.

This requires 'static.

With a non-'static non-Rc type it seems a bit harder, but it looks like a generally safe API that creates UB in this case can be created: specifically a TLS API storing Option that are forgotten instead of freed where the lifetime-bound key can be sent between threads and thus sent back to the original thread after the future is moved to a different thread.

You'd need to give more detail, but I don't think this allows anything: if the TLS value can be accessed outside of the future, that means the reference must have a lifetime which is visible in the signature of that future, so the compiler can easily see that it escapes and not implement Send in that case.

Hmm, that is certainly a more challenging example. To address that it seems like you'd need to introduce some kind of extra marker trait, or system for determining the origin of references.

I don't think this is worth the complexity cost, references and lifetimes are confusing enough for people to learn without this. Adding special cases will just make things harder for minimal gains in a niche use case.

If it required changes to non-async functions then yes. However, I don't agree that this is a niche use-case: I think the main reason it hasn't been hit often is that async/await is still relatively new. Within async functions, this example crops up a lot: even trivial examples hit this problem when translating from synchronous code to async/await.

See also this old issue around this:

2 Likes