Why isn't `unsafe impl<T: Sync> Sync enough for Arc<T>`?

1

Send and Sync is implemented as follows for Arc<T>:

unsafe impl<T: ?Sized + Send + Sync> Send for Arc<T> {}
unsafe impl<T: ?Sized + Send + Sync> Sync for Arc<T> {}

I have created some counterexamples to show that the following bounds are necessary (making miri show there is UB by relaxing any of the 3):

unsafe impl<T: ?Sized + Send + Sync> Send for Arc<T> {}
unsafe impl<T: ?Sized + Sync> Sync for Arc<T> {}

However, I can't come up with a counterexample for why we also need T to be Send for Arc<T> to be Sync, if we still require the 3 other bounds.

As discussed in Why T: Sync does not means Arc<T>: Sync it is apparently possible to send the T to another thread by using Arc::try_unwrap.
However, I can't seem to come up with an example where this is possible.

Sure we can create an Arc<T> and a reference to it in one thread, send the reference to another thread and obtain another Arc<T> in that thread.
However, the reference needs to still be valid for the other thread, so the original Arc<T> can't be dropped before the thread is done running.

So I can't see how to make Arc::try_unwrap succeed in the second way.

2 Likes
2

This is false. A simple counterexample:

  1. Thread A construct x: Arc<T> then spawn thread B with &x captured.
  2. Then thread B spawns thread C with y = Arc::clone(&x) captured.
  3. Then thread A & B terminates.
  4. Then thread C unwraps y successfully.
3

2 isn't possible because Arc<T> is not Send.

2 Likes
4

Yeah, you are correct, Arc<T> is not Send!

I guess the problem is that, the reference needs to still be valid for the other thread is not really guaranteed at language level. (std::thread::scope is a library API). I will try to come up with a better counter example when I got time.

5

It doesn't matter that Arc isn't Send here, and thread C isn't needed to cause a problem. 2 is problematic because it is cloning the Arc, and it is the fact that Arc can be cloned to gain ownership from a shared reference that means Send needs to be required for it to be Sync. Consider the following:

  1. Thread A constructs x: Arc<T> then spawns thread B with &x captured.
  2. Thread B calls Arc::clone() to get an owned Arc<T>.
  3. Thread A drops x. Thread B's clone is now the only remaining Arc pointing to the original T.
  4. Thread B terminates or otherwise drops its Arc, the T is then dropped on the wrong thread (this can matter for something like MutexGuard, which is not Send).
1 Like
6

How can you drop x in thread A, when thread B has a reference to x which is owned by thread A?

This doesn't seem possible to achieve (without unsafe code).

7

EDIT: this example is wrong.

let foobar = ...;
let x = Arc::new(foobar);
thread::scope(|s| {
    s.spawn(|| {
        let y = x.clone();
        thread::spawn(move || {
            do_some_work(y)
        })
    })
});
1 Like
8

I thought about this too, but when you move y to the other thread you need the Arc to be Send and Send still has the T: Send requirement.

9

you are right, I forgot about that.

it seems to me, although we can get an owned Arc by cloning, the cloned Arc cannot escape the scope of the borrow's lifetime without Send. so I tend to agree with you, maybe T: Send is unnecessary for Arc<T>: Sync? that's an interesting discovery.

2 Likes
10

Found some old discussion on this: `Arc` should only require `Sync`, not `Send` · Issue #20257 · rust-lang/rust · GitHub

1 Like
11

The following code demonstrates how you could obtain a T on another thread:

struct ArcSyncWithoutSendBound<T>(#[allow(unused)] Arc<T>);
unsafe impl<T: Sync> Sync for ArcSyncWithoutSendBound<T> {}
impl<T> Clone for ArcSyncWithoutSendBound<T> {
    fn clone(&self) -> Self {
        Self(self.0.clone())
    }
}

struct SendPointer<T: ?Sized>(*const T);
unsafe impl<T: ?Sized + Sync> Send for SendPointer<T> {}

static MUTEX: LazyLock<Mutex<Box<i32>>> = LazyLock::new(|| Mutex::new(Box::new(1)));

let guard = MUTEX.lock();

let x = ArcSyncWithoutSendBound(Arc::new(guard));
let x_ref = &x;
let ref_dropped = Arc::new(AtomicBool::new(false));
{
    let ref_dropped = ref_dropped.clone();
    let x_ptr_wrapped = SendPointer(&raw const *x_ref);
    thread::spawn(move || {
        let mut y = {
            let x_ptr_wrapped = x_ptr_wrapped;
            let x_ref = unsafe { x_ptr_wrapped.0.as_ref().unwrap() };
            x_ref.clone()
        };

        ref_dropped.store(true, Ordering::SeqCst);
        loop {
            match Arc::try_unwrap(y.0) {
                Ok(_) => {
                    println!("Dropped in wrong thread!");
                    break;
                }
                Err(y_old) => {
                    y = ArcSyncWithoutSendBound(y_old);
                }
            }
        }
    });
}
while !ref_dropped.load(Ordering::SeqCst) {
    thread::sleep(Duration::from_millis(10));
}
drop(x);
12

It would still be interesting if someone could come up with an example without any extra unsafe code such as SendPointer and x_ptr_wrapped.0.as_ref().

13

Send is required.

  1. Create Arc<T> in thread A.
  2. Send &Arc<T> to thread B.
  3. Call clone in thread B to obtain a second Arc<T>.
  4. Drop the Arc<T> in thread A.
  5. Drop the Arc<T> in thread B.

This causes the destructor of T to run on thread B even though the value was created in thread A. That's only allowed if T is Send.

1 Like
14

You could use rayon for an example. It offers a thread-pool with a scoped-style API, but the threads live on for longer.

3 Likes
15

Again: Step 4 is not really possible, when thread B has a reference to the Arc<T> created by thread A.

(Of course with unsafe and some other synchronization you could do it).

16

Not necessarily with thread::scoped, but it is possible e.g. with rayon.

17

The scoped thread API may not provide any easy way to do it, but what I described is still allowed and follows all of the rules. And in fact @steffahn already shared an example of how you can do it by having the second Arc<T> escape via a thread-local.

2 Likes
18

Or an easier example:

use std::sync::{Arc, Weak};

fn main() {
    let my_arc: Arc<T> = ...;
    
    let weak: &'static Weak<T> = Box::leak(Box::new(my_arc.downgrade()));
    
    std::thread::spawn(move || {
        let arc = weak.upgrade();
        drop(arc);
    });
    drop(arc);
}

This will drop them in parallel, so it'll only happen sometimes. If you prefer, you can add some barriers to make it happen every time.

4 Likes
19

I thought about thread local, but aren't thread locals supposed to be dropped when threads terminate?

20

@nerditation The thread did not terminate. It was reused to run another closure because rayon utilizes a thread-pool.