Add Rc::borrow_mut(&mut Self)

[finnbear]

Proposal

Problem statement

It is impossible to get a mutable reference into an Rc<T> when the weak reference count is non-zero, even when the strong reference count is 1.

Motivation, use-cases

Right now, if I want to share and mutate data in my single-threaded Yew/Webassembly application, I have to use Rc<RefCell<T>>. Unfortunately, the RefCell means that all immutable borrows require a Ref guard. As I originally designed my application around simply Cloneing the state to share it, my hundreds of immutable accesses are via de-referencing and would all need to be adapted to use a Ref guard.

If this proposal is implemented, a guard is only required during mutation. Immutable accesses can take place both by de-referencing (e.g. the synchronous business logic part of my app) and, in the same program, sharing Weak<T>'s that get upgraded to Rc<T> before the access takes place (e.g. the asynchronous Yew component hierarchy part of my app).

Solution sketches

Add std::rc::RefMut.

Then, either:

• Add Rc<T>::borrow_mut(&mut Self) -> Option<std::rc::RefMut<'_, T>> (consistent with Rc::get_mut returning an Option)
• Add Rc<T>::borrow_mut(&mut Self) -> std::rc::RefMut<'_, T> and Rc<T>::try_borrow_mut(&mut Self) -> Option<std::rc::RefMut<'_, T>> (consistent with RefCell::borrow_mut and RefCell::try_borrow_mut)

The above APIs can be implemented by:

1. Taking a &mut Self so that it can't be cloned during the process
2. Checking if the strong reference count is 1. If not, there are other Rc's, so panic or return None.
3. Setting the strong reference count to zero (so existing Weak's cannot upgrade)
4. Returning a guard that impl Deref<Target = T> + DerefMut
5. The guard impl Drop to restore the strong reference count to 1

Links and related work

I implemented this here. It uses unsafe and relies on unspecified implementation details of Rc (meaning it would benefit from inclusion in std), but passes tests under Miri and works in my application.

What happens now?

This issue is part of the libs-api team API change proposal process. Once this issue is filed the libs-api team will review open proposals in its weekly meeting. You should receive feedback within a week or two.

[m-ou-se]

The documentation of Weak::upgrade() currently says:

Returns None if the inner value has since been dropped.

But with this change, it could also return None while the value hasn't been dropped yet, but is just borrowed out mutably through RefMut. I'm not sure if that's a big problem, but I can imagine code that might make assumptions about Weak::upgrade() returning None meaning that the original object is fully gone.

If this proposal is implemented, a guard is only required during mutation. Immutable accesses can take place both by de-referencing (e.g. the synchronous business logic part of my app) and, in the same program, sharing Weak's that get upgraded to Rc before the access takes place (e.g. the asynchronous Yew component hierarchy part of my app).

Using that pattern, everything that only needs immutable access then needs to use a Weak<T>, such that requesting access (like RefCell::borrow()) would go through Weak::upgrade(). But that also means that if the only users left are those who need only immutable access, there are only weak pointers left, and the T gets dropped and deallocated. That seems wrong?

I agree that using a Rc<RefCell<T>> can be a bit verbose with .borrow() everywhere, but using a Weak<T> with Weak::upgrade() everywhere doesn't seem much better.

I implemented this here.

Note that your implementation breaks and can result in a use-after-free when leaking a RefMut:

use std::rc::Rc;
use rc_borrow_mut::RcBorrowMut;

fn main() {
    let mut rc = Rc::new("asdf".to_string());
    std::mem::forget(Rc::borrow_mut(&mut rc));
    drop(rc.clone());
    println!("use after free: {rc:?}");
}

$ cargo run
use after free: "@��q"

[finnbear]

I agree that using a Rc<RefCell> can be a bit verbose with .borrow() everywhere, but using a Weak with Weak::upgrade() everywhere doesn't seem much better.

Minor correction: Rc<RefCell<T>> does force you to use .borrow() everywhere but, under this proposal, immutable accesses can take place via impl Deref for Rc wherever &rc is within reach (i.e. by the owner). Weak is required for parts of the code that do not have a &rc.

But that also means that if the only users left are those who need only immutable access, there are only weak pointers left, and the T gets dropped and deallocated. That seems wrong?

Thanks for pointing this out! That's not ideal.

Note that your implementation breaks and can result in a use-after-free when leaking a RefMut:

Thanks for pointing this out! 😨

While I knew not to trust destructors, I was under the (false) impression that code like this wouldn't compile:

let mut vec = vec![1, 2, 3];
std::mem::forget(vec.iter_mut());
println!("{vec:?}");

Fixing this would require taking ownership of the Rc (weird and not sure what gets put in its place if called on a struct field) or adding some 'sentinel' value to cause Rc::clone to fail somehow, I no longer believe this feature is worth including in std. I'll also archive the repository on GitHub and add a warning comment.