Is it safe to `Send` struct containing `Rc` if strong_count is 1 and weak_count is 0?
Asked Answered
D

2

10

I have a struct that is not Send because it contains Rc. Lets say that Arc has too big overhead, so I want to keep using Rc. I would still like to occasionally Send this struct between threads, but only when I can verify that the Rc has strong_count 1 and weak_count 0.

Here is (hopefully safe) abstraction that I have in mind:

mod my_struct {
    use std::rc::Rc;

    #[derive(Debug)]
    pub struct MyStruct {
        reference_counted: Rc<String>,
        // more fields...
    }

    impl MyStruct {
        pub fn new() -> Self {
            MyStruct {
                reference_counted: Rc::new("test".to_string())
            }
        }

        pub fn pack_for_sending(self) -> Result<Sendable, Self> {
            if Rc::strong_count(&self.reference_counted) == 1 &&
               Rc::weak_count(&self.reference_counted) == 0
            {
                Ok(Sendable(self))
            } else {
                Err(self)
            }
        }

        // There are more methods, some may clone the `Rc`!
    }

    /// `Send`able wrapper for `MyStruct` that does not allow you to access it,
    /// only unpack it.
    pub struct Sendable(MyStruct);

    // Safety: `MyStruct` is not `Send` because of `Rc`. `Sendable` can be
    //         only created when the `Rc` has strong count 1 and weak count 0.
    unsafe impl Send for Sendable {}

    impl Sendable {
        /// Retrieve the inner `MyStruct`, making it not-sendable again.
        pub fn unpack(self) -> MyStruct {
            self.0
        }
    }
}

use crate::my_struct::MyStruct;

fn main() {
    let handle = std::thread::spawn(|| {
        let my_struct = MyStruct::new();
        dbg!(&my_struct);

        // Do something with `my_struct`, but at the end the inner `Rc` should
        // not be shared with anybody.

        my_struct.pack_for_sending().expect("Some Rc was still shared!")
    });

    let my_struct = handle.join().unwrap().unpack();
    dbg!(&my_struct);
}

I did a demo on the Rust playground.

It works. My question is, is it actually safe?

I know that the Rc is owned only by a single onwer and nobody can change that under my hands, because it can't be accessed by other threads and we wrap it into Sendable which does not allow access to the contained value.

But in some crazy world Rc could for example internally use thread local storage and this would not be safe... So is there some guarantee that I can do this?

I know that I must be extremely careful to not introduce some additional reason for the MyStruct to not be Send.

Doha answered 21/11, 2019 at 14:28 Comment(0)
V
11

No.

There are multiple points that need to be verified to be able to send Rc across threads:

  1. There can be no other handle (Rc or Weak) sharing ownership.
  2. The content of Rc must be Send.
  3. The implementation of Rc must use a thread-safe strategy.

Let's review them in order!

Guaranteeing the absence of aliasing

While your algorithm -- checking the counts yourself -- works for now, it would be better to simply ask Rc whether it is aliased or not.

fn is_aliased<T>(t: &mut Rc<T>) -> bool { Rc::get_mut(t).is_some() }

The implementation of get_mut will be adjusted should the implementation of Rc change in ways you have not foreseen.

Sendable content

While your implementation of MyStruct currently puts String (which is Send) into Rc, it could tomorrow change to Rc<str>, and then all bets are off.

Therefore, the sendable check needs to be implemented at the Rc level itself, otherwise you need to audit any change to whatever Rc holds.

fn sendable<T: Send>(mut t: Rc<T>) -> Result<Rc<T>, ...> {
    if !is_aliased(&mut t) {
        Ok(t)
    } else {
        ...
    }
}

Thread-safe Rc internals

And that... cannot be guaranteed.

Since Rc is not Send, its implementation can be optimized in a variety of ways:

  • The entire memory could be allocated using a thread-local arena.
  • The counters could be allocated using a thread-local arena, separately, so as to seamlessly convert to/from Box.
  • ...

This is not the case at the moment, AFAIK, however the API allows it, so the next release could definitely take advantage of this.


What should you do?

You could make pack_for_sending unsafe, and dutifully document all assumptions that are counted on -- I suggest using get_mut to remove one of them. Then, on each new release of Rust, you'd have to double-check each assumption to ensure that your usage if still safe.

Or, if you do not mind making an allocation, you could write a conversion to Arc<T> yourself (see Playground):

fn into_arc<T>(this: Rc<T>) -> Result<Arc<T>, Rc<T>> {
    Rc::try_unwrap(this).map(|t| Arc::new(t))
}

Or, you could write a RFC proposing a Rc <-> Arc conversion!

The API would be:

fn Rc<T: Send>::into_arc(this: Self) -> Result<Arc<T>, Rc<T>>

fn Arc<T>::into_rc(this: Self) -> Result<Rc<T>, Arc<T>>

This could be made very efficiently inside std, and could be of use to others.

Then, you'd convert from MyStruct to MySendableStruct, just moving the fields and converting Rc to Arc as you go, send to another thread, then convert back to MyStruct.

And you would not need any unsafe...

Vigesimal answered 22/11, 2019 at 16:25 Comment(0)
R
1

The only difference between Arc and Rc is that Arc uses atomic counters. The counters are only accessed when the pointer is cloned or dropped, so the difference between the two is negligible in applications which just share pointers between long-lived threads.

If you have never cloned the Rc, it is safe to send between threads. However, if you can guarantee that the pointer is unique then you can make the same guarantee about a raw value, without using a smart pointer at all!

This all seems quite fragile, for little benefit; future changes to the code might not meet your assumptions, and you will end up with Undefined Behaviour. I suggest that you at least try making some benchmarks with Arc. Only consider approaches like this when you measure a performance problem.


You might also consider using the archery crate, which provides a reference-counted pointer that abstracts over atomicity.

Ryon answered 21/11, 2019 at 15:28 Comment(5)
While I agree that using Arc may be the easiest solution possibly with no or small performance hit, this does not answer my question. My question is: "Is the code I posted safe? If not, why?" You are not correct about not being able to clone the Rc at all - you may clone it, then drop the clones, then you can verify that they are gone using strong_count and weak_count.Doha
Actually if you can give me source for the statement "If you have never cloned the Rc, it is safe to send between threads.", then that would most likely answer my question.Doha
@Doha (apart from this source), it's just logic: there is no risk of data races on a variable that doesn't change, and which is only accessible in one thread!Ryon
That is true in today's implementation. But it could change tomorrow. Some types, e.g. Box or Vec provide guarantees that the internals will remain the same, I am not sure about Rc. But I guess it is mostly safe to assume it won't change to something crazy (e.g. using thread local storage).Doha
@Doha The principle of an Rc is not complex. It's difficult to imagine the fundamentals of the implementation changing.Ryon

© 2022 - 2024 — McMap. All rights reserved.