Safely circumventing 1 mutable XOR 1..n immutable references to RefCell requirement

I saw an interesting (as of when I posted this topic still unanswered) question on SO today:

which is encountering a panic in RefCell::borrow_mut, because the RefCell is already borrowed. The model of the OPs program will always panic this way, because it is trying to get a mutable reference to the RefCell through an immutable reference.

Now, I think we can safely circumvent the panic by mutating the value inside the RefCell (which is held inside an Rc) through a raw *mut T pointer. I'm not confident in my unsafe Rust skills and wanted to check in with the community and see if my reasoning behind why I think this will be safe is correct. I couldn't find anything that would ease my mind in the ownership section of the nomicon. This comment by @jdahlstrom comes very close to describing what I want to achieve and I think my code meets the criteria.

Here is what I'm talking about:

use std::{
    cell::RefCell,
    rc::{Rc, Weak},
};

// parent inner
struct I {
    c: C,
    b: B,
}

// child
struct C {
    i: Weak<RefCell<I>>,
    v: String,
}

// child
struct B {
    i: Weak<RefCell<I>>,
    v: String,
}

// parent
struct P {
    i: Rc<RefCell<I>>,
}

impl P {
    fn new() -> P {
        let b = B {
            v: "b".to_string(),
            i: Weak::new(),
        };
        let c = C {
            v: "c".to_string(),
            i: Weak::new(),
        };
        let i = Rc::new(RefCell::new(I { b, c }));
        let ii = i.clone();
        let p = P { i };

        // init b.i
        let mut borrow_mut = RefCell::borrow_mut(&ii);
        let bb = &mut borrow_mut.b;
        bb.i = Rc::downgrade(&ii);

        // init c.i
        let cc = &mut borrow_mut.c;
        cc.i = Rc::downgrade(&ii);
        p
    }

    fn update_bv_cv(&self) {
        let mut borrow_mut = RefCell::borrow_mut(&self.i);
        
        // update c.v directly
        borrow_mut.c.v.push_str("=>p.update_bv_cv");
    
        // update c.v through to b
        
        // SAFETY:
        //
        // There aren't any data races (Rc is neither Send nor Sync), so 
        // dereferencing the raw pointer to `RefCell` here should be safe, too, 
        // right?
        //
        // Also we drop the raw pointer immediately so it doesn't live beyond the 
        // scope of this function, so we can't call `RefCell::borrow_mut` again
        // while a raw pointer to the underlying data exist.
        //
        unsafe { (*borrow_mut.b.i.upgrade().unwrap().as_ptr()).c.v.push_str(" b.update_cv"); }
    }

    fn get_bv_cv(&self) -> (String, String) {
        let i = &self.i;
        let ii = i.borrow();

        let bv = ii.b.v.as_str();
        let cv = ii.c.v.as_str();

        (bv.into(), cv.into())
    }
}


fn main() {
    let p = P::new();
    p.update_bv_cv();
    let (bv, cv) = p.get_bv_cv();
    println!("b.v: {bv}\nc.v: {cv}");
}

Playground.

I ran miri on the playground and it doesn't complain and the program runs as expected. But I'm wondering if I've missed anything.

Seems like switching to UnsafeCell would be simpler?

I'm not sure, but it looks to me like this is converting a ref to a mut ref though a pointer cast, which I believe is immediate UB, I'm not sure why miri isn't flagging it.

1 Like

I can't tell if using UnsafeCell directly in favour of RefCell will be feasible, as I haven't written the example.

... I don't know :sweat_smile:. We already hold the only mutable reference to the value I inside RefCell in the borrow_mut variable. We use that to get the field b.i of I which is itself a Weak reference to the RefCell where I is stored. We then convert the Weak pointer to a raw pointer to get a *const RefCell<I>[1]. We upgrade the Weak pointer and dereference it to get to RefCell<I> and convert that to a raw pointer *mut I. We have to, because we used our mutable reference to I to get to that Weak pointer we want to use to get another mutable reference to I. That will panic in safe Rust, because RefCell prevents us from having more than one mutable reference (or one mutable reference and n immutable ones). We dereference the raw pointer to mutate I.


  1. I may have overcomplicated things here, as we could use Weak::upgrade here, which would be a safe alternative. ↩︎

1 Like

This works

let i = {
    // b update c.v
    let ref_i = RefCell::borrow(&self.i); // RefCell::<I>::borrow::<'r>(&'r self) -> Ref<'r, I>
    ref_i.b.i.clone()
};
update_cv_from_b(&i);

impl B {
    fn update_cv(&self) {
        update_cv_from_b(&self.i);
    }
}

fn update_cv_from_b(i: &Weak<RefCell<I>>) {
    if let Some(i) = i.upgrade() {
        let mut ii = RefCell::borrow_mut(&i);
        ii.c.v.push_str("b.udpate_cv");
    }
}

Rust Playground

1 Like

Very nice how you circumvent using unsafe at all by dropping the reference to RefCell inside update_bv_cv! I haven't thought of this. You should post that as an answer to the original question on SO.

I'm still curious if my unsafe version is UB though.

It seems to rely on some implementation details of BorrowMut though (combined with the fact that the target is no longer accessed after the unsafe code). If – instead of re-entering through the borrow_mut itself every time – keep s single mutable reference around, then the code will conflict with / invalidate that reference. I.e.

    fn update_bv_cv(&self) {
        let mut borrow_mut = RefCell::borrow_mut(&self.i);

+       let borrow_mut = &mut *borrow_mut; // access through mutable reference
        
        // update c.v directly
        borrow_mut.c.v.push_str("=>p.update_bv_cv");
    
        // update c.v through to b
        unsafe { (*borrow_mut.b.i.upgrade().unwrap().as_ptr()).c.v.push_str(" b.update_cv"); }
+       borrow_mut; // access the mutable reference after the unsafe code
        // BOOM
    }
error: Undefined Behavior: trying to retag from <4442> for Unique permission at alloc1874[0x20], but that tag does not exist in the borrow stack for this location
  --> src/main.rs:75:9
   |
75 |         borrow_mut;
   |         ^^^^^^^^^^
   |         |
   |         trying to retag from <4442> for Unique permission at alloc1874[0x20], but that tag does not exist in the borrow stack for this location
   |         this error occurs as part of retag at alloc1874[0x18..0x58]
   |
   = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental

If you already saw through these subtleties, congratulations – otherwise – maybe avoid this kind of code, as it doesn’t seem to be sound reliably enough to me.

1 Like

Thank you, very interesting. No I haven't thought of this and I haven't tried using &mut I instead of RefMut<'_, I>. That seems quite brittle indeed.

Glad to see you like it. :handshake: I've put my solution there. I don't know much about unsafe Rust, so I can't give more comments.

1 Like

For normal shared references, it's true. But the shared reference to types based on UnsafeCell will likely be considered as *mut.

given a reference type, a tag, and whether we are inside an UnsafeCell , we can compute the matching Borrow : Mutable references use Mut(Uniq(tag)) , shared references in an UnsafeCell use Mut(Raw) and other shared references use Frz(tag)
src: https://www.ralfj.de/blog/2018/08/07/stacked-borrows.html#52-mir-operations

Based on the doc example of RefCell::as_ptr, this is not UB: Rust Playground

let c = RefCell::new(5);
let ptr = c.as_ptr();
unsafe { *ptr = 6 }; // not UB
dbg!(&c);            // not UB: even you use the reference to c here
unsafe { *ptr = 7 }; // not UB

Updated...

and interestingly, we can move the mutable borrow to a local scope to prevent the code from UB Rust Playground

let mut borrow_mut = RefCell::borrow_mut(&self.i);
{
  let borrow_mut = &mut *borrow_mut; 
  borrow_mut.c.v.push_str("=>p.update_bv_cv");
  unsafe { (*borrow_mut.b.i.upgrade().unwrap().as_ptr()).c.v.push_str(" b.update_cv"); }
}
let _ = &mut *borrow_mut;   // not UB
let _ = &mut borrow_mut.b;  // not UB
1 Like

Yes that is plausible to me. Thanks for your example. It makes it easier to get an error message I can understand from miri for @steffahn's example:

fn main() {
    use std::cell::RefCell;

    let c = RefCell::new(5);

    let borrow: &mut i32 = &mut *c.borrow_mut();

    let ptr = c.as_ptr();

    unsafe { *ptr = 6 };
    
    borrow;
}

Playground.

Miri error message
error: Undefined Behavior: trying to retag from <3111> for Unique permission at alloc1584[0x8], but that tag does not exist in the borrow stack for this location
  --> src/main.rs:12:5
   |
12 |     borrow;
   |     ^^^^^^
   |     |
   |     trying to retag from <3111> for Unique permission at alloc1584[0x8], but that tag does not exist in the borrow stack for this location
   |     this error occurs as part of retag at alloc1584[0x8..0xc]
   |
   = help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
   = help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
help: <3111> was created by a Unique retag at offsets [0x8..0xc]
  --> src/main.rs:6:28
   |
6  |     let borrow: &mut i32 = &mut *c.borrow_mut();
   |                            ^^^^^^^^^^^^^^^^^^^^
help: <3111> was later invalidated at offsets [0x8..0xc] by a write access
  --> src/main.rs:10:14
   |
10 |     unsafe { *ptr = 6 };
   |              ^^^^^^^^
   = note: BACKTRACE (of the first span):
   = note: inside `main` at src/main.rs:12:5: 12:11

note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace

error: aborting due to previous error; 1 warning emitted

So using unsafe { *ptr = 6 }; invalidates any previously existing references to that location. Ref<'_, T> and RefMut<'_, T> somehow are able to circumvent invalidation (they use Cell underneath from what I could grasp, so maybe there is something going on in there?).

Very interesting (to me) indeed, but so far I only feel reinforced in my reluctance to write unsafe Rust :smile:

1 Like

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.