Using Arc::make_mut() with Arc<[T]>

I'm implementing a buffer with copy-on-write semantics and was running into some issues with the borrow checker when implementing a make_mut() method.

use std::sync::Arc;

#[derive(Debug, Clone, PartialEq)]
struct CowBuffer<T> {
    elements: Arc<[T]>,
}

impl<T: Clone> CowBuffer<T> {
    pub fn make_mut(&mut self) -> &mut [T] {
        // Note: we can't use Arc::make_mut() because [T] is not Clone

        if let Some(elements) = Arc::get_mut(&mut self.elements) {
            return elements;
        }

        // Looks like more than one person has access to our elements
        // so we need to make a copy.
        self.elements = self.elements.iter().cloned().collect();
        Arc::get_mut(&mut self.elements).expect("Guaranteed to be unique")
    }
}

And the compile error:

error[E0506]: cannot assign to `self.elements` because it is borrowed
  --> src/lib.rs:18:9
   |
9  |     pub fn make_mut(&mut self) -> &mut [T] {
   |                     - let's call the lifetime of this reference `'1`
...
12 |         if let Some(elements) = Arc::get_mut(&mut self.elements) {
   |                                              ------------------ borrow of `self.elements` occurs here
13 |             return elements;
   |                    -------- returning this value requires that `self.elements` is borrowed for `'1`
...
18 |         self.elements = self.elements.iter().cloned().collect();
   |         ^^^^^^^^^^^^^ assignment to borrowed `self.elements` occurs here

(playground)

I understand why the error occurs - the borrow in that first Arc::get_mut() call is similar to mutating self-referential types in that we "lock" self for the duration of the method call. That means any further uses (like re-assigning self.elements later on) will fail because self is already mutably borrowed.

My question is whether there are any workarounds I can use without changing the method signature?

I could always switch to a with_mut() method that takes a FnMut(&mut [T]) closure, but that isn't quite as ergonomic.

You could check the strong_count() and weak_count() yourself to decide whether you need to do the cloning. If they’re both 0, the get_mut will succeed, and there’s no way for other code to increase them:

use std::sync::Arc;

#[derive(Debug, Clone, PartialEq)]
struct CowBuffer<T> {
    elements: Arc<[T]>,
}

impl<T: Clone> CowBuffer<T> {
    pub fn make_mut(&mut self) -> &mut [T] {
        // Note: we can't use Arc::make_mut() because [T] is not Clone

        if Arc::strong_count(&self.elements) > 0
        || Arc::weak_count(&self.elements) > 0 {
            self.elements = self.elements.iter().cloned().collect();
        }

        Arc::get_mut(&mut self.elements).expect("Guaranteed to be unique")
    }
}

Playground

Edit: This code is broken; see below.

Oh, of course. Thanks @2e71828!

Similarly, you could use

if let Some(_) = Arc::get_mut(&mut self.elements) {
    return Arc::get_mut(&mut self.elements).expect("Guaranteed to be unique");
}

this makes the rest of your original code compile.

1 Like

I just realized that my code is at least slightly incorrect: It needs to check for strong_count>1, or else it will always clone. I also don’t know if there’s a race condition: Can an Arc <-> Weak conversion in another thread dodge between the two checks somehow?

I would believe it can. The implementation of get_mut uses some fancy “locking” of the weak count to soundly take both counts into account. I’m not fully understanding the situation though.

1 Like

I've got a feeling reading and checking the weak and strong counts at different times will leave you open to a race where someone quickly upgrades or downgrades to get a second reference to the same Arc when we call Arc::get_mut().expect().

It's not a safety issue per-se but I'd prefer to avoid a the possibility of panicking, even if that should only ever happen in a contrived/malicious/unlucky scenario.

1 Like