RefCell - inconsistency in the docs or some magic?


#1

Ok so again I have the following code:

use std::cell::{Ref, RefMut, RefCell};
use std::rc::Rc;
use std::ops::Deref;

#[derive(Debug, Clone, Default)]
struct A {
    id: usize,
    x: Option<i32>
    // ** more fields **
}

#[derive(Debug, Clone, Default)]
struct B {
    v: Vec<RefCell<A>>
}

#[derive(Debug, Clone, Default)]
struct C {
    rc: Rc<RefCell<B>>,
}

#[derive(Debug, Clone, Default)]
struct D {
    c: C,
    id: usize
}

impl D {
    // pub fn get(&self) -> Result<Ref<A>, String> {
    //     if self.c.rc.borrow().v.get(self.id).is_none() {
    //         Err("not present".into())
    //     }  else {
    //         Ok(Ref::map(self.c.rc.borrow(), |x| {
    //             x.v.get(self.id).unwrap().borrow().deref()
    //         }))
    //     }
    // }

    pub fn get_mut(&self) -> Result<RefMut<A>, String> {
        if self.c.rc.borrow().v.get(self.id).is_none() {
            Err("not present".into())
        }  else {
            Ok(RefMut::map(self.c.rc.borrow_mut(), |x| {
                x.v.get_mut(self.id).unwrap().get_mut()
            }))
        }
    }
}

fn main() {
    let c = C::default();
    c.rc.borrow_mut().v.push(RefCell::new(A::default()));
    c.rc.borrow_mut().v.push(RefCell::new(A::default()));
    let d = D{c: c.clone(), id:1};
    d.get_mut().unwrap().x = Some(13);
    println!("{:?}", c.rc.borrow().v);
    
}

The get_mut works as a charm, however I really did not find any way of implementing equivalently the get. If I use the borrow() and deref() the compiler is complaining about the temporary borrow. However, somehow the get_mut of in the RefMut case works, which according to the docs borrows the RefCell mutably, but somehow makes no temporary?? How is this possible and how can I correctly implement the get method?


#2

This is a really interested question! I haven’t understood your code fully yet (I’ve never used Ref::map method before, and the nesting level of RefCells is quite deep), but I can give you one advice that may be helpful:

It is usually easier to put the RefCell inside the struct (to wrap the individual field). In your case (changing only RefCell around the A struct). Full playground

#[derive(Debug, Clone, Default)]
struct A {
    id: usize,
    x: Cell<Option<i32>>
    // ** more fields **
}

The other advice is to use Cell over RefCell where you can – in this example using Cell even made the get_mut unnecessary.


#3

Ah thanks for the interested in at least trying this :slight_smile:

So on the topic - the code is a very dumbed down example. In the real code I actually have things there that are non copyable, which is why it is in RefCell rather than Cell. The Ref::map basically allows you to extract references to inner bits of the struct. Lastly, I don’t want to put RefCell on each field, as the struct is such that I either modify many of them together or don’t modify anything, thus semantically it makes sense to have a RefCell on the struct. The reason for why that is there in the first place is that I need to be able to modify different bits of the vector potentially at the same time.
But yes, to me is really a misery why I can do the get_mut but not the get


#4

Now I’ve read your original code one more time and had an enlightenment – why the get was impossible in your original code.

In the get_mut method, you do return a RefMut<A>. This RefMut symbolizes a one uniquely locked RefCell. In your case, this is a locked RefCell<B> inside Rc inside C struct. Since this is an unique lock, it means that nobody else can have access to this B struct now. Now, you have to access the RefCell<A> inside the vec. And you use the RefCell::get_mut for that – remember, that now you have an unique access to B and when you have unique access to the object containing the RefCell, no locking is necessary and you can just access the interior. So no lock necessary for A.

Now, in the case of get method, you’d need to lock the RefCell<B> for reading (by .borrow()). That means a shared access to B. But shared access to B doesn’t prevent somebody else from gaining an mutable access to A! – it’s behind another RefCell. So you need an additional lock on RefCell<A>. So you’d want to return an object that represents two locks. But you’re returning a single Ref (which represents one lock). That’s why it doesn’t work.


You should also think if you really need a RefCell layer over the A struct. Do you need to simultaneously have active borrows over different A structs? (I mean mutable + mutable or mutable + shared). If you plan using the A struct indirectly via your D::get_mut, the whole vector would be mutably borrowed anyway and the RefCell around A doesn’t buy you anything.

If you really want simultaneous mutable accesses to A then I think it’s impossible (or really hard, at least in safe Rust) to return a struct that represents multiple layers of locked RefCells. The simplest solution would be to leave only get method, and return Ref<RefCell<A>> and let the caller call that one additional borrow_mut. The thing I mentioned before about putting RefCell inside the struct was mainly to make API cleaner (so you can return “just” Ref<A>), and let the A contain RefCell<AInner>).


#5

Ok so the goal is to be able to modify different nodes in the vector v in B. This is currently possible via the get_mut, which actually I think means that what you said is not correct, as the returned value does not lock B, but locks each A independently, e.g. this https://is.gd/ZQ8Uvr compiles and works perfectly. E.g. this does not give you a unique access to B.

I actually personally do not even need the get method really, I can use mutable reference as well. However, for a library and Rust code recommendations it is good to have a get as well. Also given that this arose, I really do not see a proper reason why get_mut works (not locking B) but I can not implement a `get.

PS: I actually want to understand what is going own, its more of a personal itch. I hate if there is smth which I can not explain - can’t let it go.


#6

Ok so the goal is to be able to modify different nodes in the vector v in B. This is currently possible via the get_mut, which actually I think means that what you said is not correct, as the returned value does not lock B, but locks each A independently, e.g. this https://is.gd/ZQ8Uvr compiles and works perfectly. E.g. this does not give you a unique access to B.

No, it locks B (while yielding references to A). See the same code with no RefCell over Aplayground. Yours and mine code works because you’re not accessing different As simultaneously, so it’s not a problem. If you really tried the simultaneous access, it would panic. I think this convinces you that the B is locked here :slight_smile:

Also given that this arose, I really do not see a proper reason why get_mut works (not locking B) but I can not implement a `get.

So the short answer is because get_mut requires only one lock (on B) and get requires two (B and A) and Ref can represent only one held lock. I think that my explanation in the post above is quite clear. If there’s something not clear enough, please tell me, I’ll try to clarify.


#7

Ok I think I got it, but then why, or more exactly how, can we achieve interior mutability on more than 1 level? In this case I do not understand the motive of why we need to lock mutably B to gain access to A, if we are changing A only internally? I thought the whole Cell/RefCell point was to allow us to do this.


#8

Well, it’s easy to get multiple levels of nested mutability, just call a chain of borrow/ borrow_muts. The hard thing is returning an nested lock. I guess it’s not in std, because it’s quite a rare usecase (although questions about this happen now and then). If you really, really, really want to return a nested lock, you can try putting multiple Refs in the same struct using unsafe or some crate. But I don’t think it’s a good idea.

Actually, we don’t. It could be possible to have a shared lock on B and unique on A. You could eg. make fn get(&self) -> Ref<RefCell<A>> and let the caller call .get().borrow_mut().x = Some(42). The only problem is returning a "nested Ref", so the caller doesn’t have to call borrow_mut(). Anyway, I think in your case the simplest way would be to get rid of one layer of RefCells, unless you’re sure you need it. You can also change the API, so there’s no need to return RefMuts – for example, you can introduce a method on D:

fn replace_a(&self, by_what: A) -> A

Not sure how it fits your problem though.