Using &mut self for a little more safety w/ interior mutability

Let's say I have something like this:

[playground]

use std::cell::RefCell;
use std::rc::Rc;

pub struct Foo {
    inner: Rc<RefCell<Bar>>,
}

pub struct Bar {}

impl Bar {
    pub fn woo(&mut self) {}
}

impl Foo {
    pub fn do_something(&mut self) {
        self.inner.borrow_mut().woo();
    }
}

fn main() {
    let mut foo = Foo {
        inner: Rc::new(RefCell::new(Bar {})),
    };

    let foo_ref_1 = &mut foo;
    // error, compiler prevents this
    // let foo_ref_2 = &mut foo;

    foo_ref_1.do_something();
}

Is that a reasonable API design?

The reason I need the interior mutability is because I have multiple Foos and each must be able to get a mutable reference to Bar, and I do not want to have to pass &mut bar all over the place, so ultimately this means multiple owners.

Since it's using interior mutability, there isn't a hard technical requirement for do_something() to take &mut self, it could just take &self, but I feel like doing it this way is just a little bit less of a footgun.

Thoughts?

If you could use &mut self, then you wouldn't need interior mutability. You can do it with the regular notion of mutability.
The idea of interior mutability is to give the ability to make borrows that work (ie, only one mutable borrow at a time, no shared borrow with a mutable borrow, etc) at runtime but cannot be proved at compile-time.
There is no footgun, since if you do an invalid borrow, the code will panic at runtime.

1 Like

I'm not sure if I understand your requirement for using a RefCell, but if you have a &mut self (for whichever reason), you may use std::cell::RefCell::get_mut. From the docs:

This method can only be called if RefCell can be mutably borrowed, which in general is only the case directly after the RefCell has been created. In these situations, skipping the aforementioned dynamic borrowing checks may yield better ergonomics and runtime-performance.

Note that if you require &mut self, then this isn't only about avoiding accidental mutation. It means that you require a exclusive reference, i.e. any other shared borrows of self must have ended. This is more likely to cause problems in your code. So if you only need &self, then I wouldn't require &mut self for the sole purpose of indicating some sort of (inner) mutation.

To demonstrate the problem:

use std::cell::RefCell;

pub struct Foo {
    name: String,
    counter: RefCell<u64>,
}

impl Foo {
    fn name(&self) -> &str {
        &self.name
    }
    fn inc(&mut self) {
        *self.counter.borrow_mut() += 1;
    }
}

fn main() {
    let mut x = Foo {
        name: "Meyer".to_string(),
        counter: RefCell::new(0),
    };
    let name = x.name();
    println!("Modifying {name}...");
    x.inc();
    // Try to uncomment the following line:
    //println!("Modification of {name} completed.");
}

(Playground)

2 Likes

This doesn't quite add up.

You say you use interior mutability in order to avoid having to pass &mut self. But then you go on to assert that you'd like to take &mut self for "extra safety". These are contradictory.

If you use interior mutability, then you don't need to take &mut self. If you want to take &mut self, then you don't need interior mutability.

Generally, if you have mutating methods, then take &mut self. Don't try to preemptively care about imaginary concerns of usability. Experienced Rust users are comfortable organizing their code so that they can provide a mutable reference if they need to. If they can't, they can wrap your type in a RefCell, but if you do that, then they can't un-wrap it. So better to leave interior mutability out of your API unless you have a really good reason not to.

5 Likes

I think your question does make sense. It is my understanding that you’ll be having multiple Foos pointing to the same object. On first thought, one might assume that &mut self methods can avoid some potential foot-gun scenarios, in particular it seems that it makes it harder to accidentally run into panics due to multiple concurrent borrow_mut calls. On second thought, it is however also quite nontrivial to run into such a panic with &self! How easy or hard this is in particular depends on your actual APIs that you’ll be having, but since this is all single-threaded, even with &self, there could only be a problem if it’s possible that in the .woo() call somehow do_something of the same Foo (or a copy of it) is called.


So all in all, I believe I haven’t seen API design like this before, and the added flexibility / ease-of-use of requiring only &self might be having more pros than cons. On the other hand, feel free to try out both (perhaps starting with the &mut self one) alternatives and see for yourself: Do any use-cases become easier to reason about when it’s &mut? Are there use-cases where you feel hindered by this restriction? Have you ever felt the need to clone a Foo for a short use-case (with the &mut API) that could’ve been using a shared reference, too? After all, Rust offers easy refactoring; change one thing and the compiler tells you all the places where you have to adapt the use-case; so trying both approaches is an option.


By the way API design where &mut on an Rc or Arc (-containing type) does make a lot of sense is when there’s no RefCells (or other interior mutability) involved, but instead you’re working with immutable data. In these cases, there is the opportunity to benefit from an optimized clone-on-write approach using Rc::make_mut: The &mut self argument for make_mut allows it to skip the cloning all together in cases where the Rc wasn’t shared after all.

6 Likes

Maybe I wasn't clear enough. It's not that I want to avoid having to pass &mut self, it's that I want to avoid having to pass &mut Bar

In other words, I am trying to avoid having to do this:

pub struct Foo { }

pub struct Bar {}

impl Bar {
    pub fn woo(&mut self) {}
}

impl Foo {
    pub fn do_something(&self, bar: &mut Bar) {
        bar.woo();
    }
}

fn main() {
    let mut bar = Bar {};
    let foo_1 = Foo { };
    let foo_2 = Foo { };

    foo_1.do_something(&mut bar);
    foo_2.do_something(&mut bar);
}

This is closer to the API I want, but it's illegal:

pub struct Foo <'a> {
    bar: &'a mut Bar 
}

pub struct Bar {}

impl Bar {
    pub fn woo(&mut self) {}
}

impl <'a> Foo <'a> {
    pub fn do_something(&mut self) {
        self.bar.woo();
    }
}

fn main() {
    let mut bar = Bar {};
   
    let mut foo_1 = Foo {
        bar: &mut bar
    };

    // not allowed!
    let mut foo_2 = Foo {
        bar: &mut bar
    };

    // but I do not need to pass &mut bar all over the place
    foo_1.do_something();
    foo_2.do_something();
}

So I figured, multiple owners / interior mutability gets me the API I want. If I'm wrong, please stop me there :slight_smile:

But if that's correct, the question then becomes if Foo is the only owner of Rc<RefCell<Bar>>, can a be a little safer by requiring that all access to the inner bar go through a &mut self...?

Hmmm good point! It just makes me feel a little better to have the compiletime checks, but you're right, I can guarantee that it's not a problem logically, and then why not have the extra flexibility of just &self

wasn't aware of Rc::make_mut(), that's cool! gotta remember that. Unfortunately my Bar type here is pretty heavy and non-Clone anyway

To clarify, it doesn't really matter if the mutable thing in question is self or another parameter, they are subject to the same rules.

If you need the Bar to be mutated from multiple different places, then you can't pass two &mut Bars at the same time, as you correctly observed. In this case, you will need interior mutability.

However, I still don't get what you want to use the &mut Bar for in this situation. If you want the API described in the above snippet, then that will never (usefully) work.

Can you tell us what these objects actually are (instead of placeholders)? Maybe that will help us suggest an idiomatic, usual alternative API design.

:+1:

&mut Bar is a third party requirement. I have no control over that.

I think the correct design here is to make it impossible to hold onto a RefMut in my API. Then I can just take &self everywhere which is a much more standard way to work with interior mutability, yeah?

Can you explain what you mean? Do you have to provide an API that demands an argument of type &mut Bar? I'm not sure if I understand.

Yes, Foo needs to call methods on Bar, and those methods are only callable on &mut Bar

Ah okay, sorry, I was confused for a short moment and now re-read your original code.

So if Bar requires a mutable (i.e. exclusive) reference, but if you want to hold multiple references at the same time (e.g. because you want to store it in an Rc or Arc, or because you store it in a struct where you have multiple shared references to), then you can use a RefCell to obtain a mutable reference at runtime (even if you only have a shared reference to the RefCell).

If you only require mutable references to Bar when you also have a mutable reference to Foo, then you wouldn't need a RefCell at all.

But let's assume you need the RefCell for now (I'm still not sure if you truly need it because I don't know the context of your application).

Now your original question was whether you should require the Foo::do_something method require &mut self. If it requires &mut self, then the function will already have the assurance that nobody else holds a reference to self (of type Foo) and thus also self.inner (of type Bar). That means self.inner.borrow_mut() could never panic, so it's okay to use self.inner.get_mut() instead, which is slightly faster (but likely not noticeable) because it doesn't do the runtime check.

I believe it's semantically okay to offer such a function if the requirement of having to provide a mutable (i.e. an exclusive reference) isn't much trouble where this method is used. But I wouldn't see this as a "safety" measure, but more as an optimization which is likely not worth to do.

You might (hypothetically) even consider adding two functions doing the same, one taking &mut self and one taking &self, similar to what RefCell does with borrow_mut and get_mut. Whenever you have a mutable reference, you can use the faster method (which doesn't do the runtime check), and when you only have a shared reference, you use the slower method.

But I feel like you either should remove the RefCell completely, or to make do_something (as well as most other methods on Foo) work with &self (because you will have multiple shared references to Foo).

Note that if you use RefCell, you might also want to consider using Mutex instead, which would make Foo be Send and Sync.

As I'm not really sure about your context, don't take these thoughts as definite advice.

3 Likes

Are you referring to the code in the original post? Because in that code, &mut self would most certainly not ensure exclusive access to *self.inner, the value that .borrow_mut() would be called on, because self.inner is an Rc.

1 Like

Oops, I missed the Rc, my apologies.

1 Like

If multiple Foos shall use the same Bar, then you need Rc (or Mutex) or shared references. Either way, this means you will also need RefCell or Mutex if you want to call methods on Bar that require &mut self. This is a given then.

Now the only question remains (if I get it right) whether Foo::do_something should require &mut self. Optimization isn't a reason here, because the Rc or shared reference to Bar will not allow skipping the runtime check via RefCell::get_mut.

The only reason I can think of is to require &mut self for future compatibility, e.g. if the inner mechanism changes, and in the future you don't store a Bar anymore but something else, which isn't shared but exclusively owned or exclusively referenced (or at least has parts that are exclusively owned or exclusively referenced).

But again, difficult to give advice here without really knowing what Foo and Bar are about.

2 Likes

Right, but if the only way to access inner was by way of the API here, then that Rc wouldn't ever be leaked.

sorta

because it needed to have been passed in, so the outside world could be cloning it and doing pretty much anything with it...

anyway, good food for thought here, thanks y'all!

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.