Why do all docs say RefCell is bad?

I started off learning rust recently. Most of the docs / articles / posts about various rust internal mutability mechanisms either directly say "RefCell is bad" or make indirect statements like "if your code has RefCell, maybe you should rethink your design etc..". As and when I understand more of Rust and write more Rust code, I cant understand the basis for such defamation of RefCell. And no one says anything bad about Arc<Mutex<>> in any of the docs or articles, and from my perspective, it has the same problems as a RefCell (explained later below). If anyone can clarify whether thats just a myth or a fact and if so why, that would give some peace of mind :).

From my perspective, the most common / frequent / practical / much-needed use case for a RefCell is as below. Lets say I have a graph structure as below for example, holding a root node and some meta data about the graph.

mod graph:

// Graph is public
pub struct Graph {
    root: Rc<RefCell<Node>>,
    depth: usize
}

impl Graph {
    pub fn add(&mut self, value: usize) {
       let r = self.root.clone().borrow_mut();
       r.add(value); 
   }
}
 
// Node is private
struct Node {
    children: Vec<Rc<RefCell<Node>>>
}

impl Node {
    fn add(&mut self, value: usize) {
    }
}

mod graph_user:

let graph = Arc::new(Mutex::new(Graph { ... }));
let graph = graph.lock().unwrap();
graph.add(100);

So the graph Node is NEVER exposed to anyone for direct use, the Node and its functions can only be used once the Graph structure itself is locked. So that means, the node is effectively accessed in a single threaded environment. Now in this case, if I were to need a mutable access to Node, what other tool do I have at disposal, what other design pattern can I choose that will avoid this.

Yes I can replace Rc<RefCell<>> with a Arc<Mutex<>> and that will probably "look" like a nice design satisfying the docs and articles, but for one its a wasted additional mutex locking AND it has the same end problems as a RefCell - if used badly, a RefCell panics, and if used badly in this example, an Arc<Mutex<>> deadlocks - now the question is whether a panic is better or a deadlock is better :).

For example, the below code obviously deadlocks, and if its a RefCell instead it will panic - both are "run time" detection of errors and not "static" detections. So back to the questions - whats wrong with using a RefCell in this situation, what else can one use if not RefCell.

struct Foo {}

fn foo(a: Arc<Mutex<Foo>>) {
    a.lock().unwrap();
}

fn main() {
    let a = Arc::new(Mutex::new(Foo{}));
    let b = a.lock().unwrap();

    foo(a.clone());
}
1 Like

Can you give an example of a place that says such things? I believe, as captain Lorca would say, that "context is king" in software engineering. I got the impression that the Rust community is not as overly-dogmatic as the C++ one, so it's interesting to know the context where such claims are made.

1 Like

It might be just my interpretation, but one example is this: Building - Learning Rust With Entirely Too Many Linked Lists. The section " When to choose interior mutability" makes me get a feeling that its something "preferably avoided, use if nothing else can work" .. And in general googling for anything and everything about rust, whenever it comes to interior-mutability, its a similar kind of feeling where its like asking "well, do you really need to do this ?". And almost always the interior mutability examples are associated with a RefCell rather than a Mutex. As far as I see it, its not like interior mutability is something unavoidable, its a very basic programming pattern which need not be talked about as "when to use it, when to avoid it" etc..

Again, it might be just my (mis)interpretation reading stuff on rust on google, but back to my question - hopefully that is just my misinterpretation ?

Rgds,
Gopa.

I mean Arc<Mutex<...>> definitely isn't an improvement compared to Rc<RefCell<...>>. The issue with both is that you easily end up with memory leaks if you have cycles of rcs. Also the runtime check is not super pretty.

One alternative is to use indexes into a vector, or using Weak instead of Rc to stop cycles from forming.

6 Likes

What you linked gives documentation about where you need or even must use RefCell. So it's only a "bad idea" to use a RefCell in the sense that it provides less guarantees than a straight-up reference. So the way I read it is this: you want to ask yourself if you really have to forgo those guarantees. Don't feel bad if the answer is "yes, I really do", but make a habit of asking.

4 Likes

Sometimes people find themselves using Rc<RefCell<T>> because they're trying to write code the way they would in a garbage-collected language, and the borrow checker is getting in the way. In a sense RefCell is a bludgeon you can use to make your design work, but it's often worse than starting from a better design and not needing it at all.

To learn more, I suggest watching Catherine West's RustConf 2018 closing keynote on using Rust in game development. Or read the blog version if you prefer. ECS isn't the be-all end-all of data-oriented design, but it's a great example of how putting a bit more thought into your data can make problems (like using RefCell everywhere) almost evaporate.

I notice that your example doesn't require interior mutability at all; Box<Node> works just as well, without changing any method signatures. (And in fact, Node alone would also work, although it's not a pointer.)

9 Likes

RefCell is “evil” in the same way that globals are “evil” and shared mutable state is “evil”: none of those things are sentient moral agents waiting for an opportunity to segfault your code when it’d cause the maximum possible client disruption in production. Rather, they are all valuable tools that fill a genuine need, but for one reason or another it’s extremely common for novices to reach for those tools when they’re not only a terrible choice, but it’s not at all obvious that they’re terrible until it’s “too late.”

Pretty much any serious attempt at addressing this reality is going to involve making the most public Rust resources loudly proclaim “this probably isn’t what you want, take a step back.” But AFAICT the official Rust docs already strike a very accurate balance here when they say “interior mutability is something of a last resort“; I don’t see much room for improvement on that page. Do you have any specific examples of docs you think are overstating the issue?

@trentj already covered why RefCell has this problem and how your example proves the point by not actually needing it.

8 Likes

If that's the talk I think it is, I think it's completely irrelevant to this discussion. She clearly shows, that she found the "optimal" solution, which is not a bunch of Rc<RefCell<T>>. However, she just got rid of the borrow checker and therefore any compile time checks. The solution is solid, but optional types and bounds checking (Vec<Optional<T>) have nothing in common with borrow checking except, that they help you write memory-safe code.

EDIT: It doesn't prevent you from indexing into something out-of-bounds, leading to a crash or receiving a None when it absolutely must be a Some (Option::unwrap), which would also lead to a crash. The borrow checker will guarantee, that you always reference something valid, but that's why it has to be conservative in many situations and in the case of game development, the borrow checker doesn't make your job that much easier.

See: https://www.youtube.com/watch?v=4t1K66dMhWk

I notice that your example doesn't require interior mutability at all ; Box<Node> works just as well, without changing any method signatures. (And in fact, Node alone would also work, although it's not a pointer.)

Thanks for the responses everyone. I will go through the talk for sure. As for the above example using Box, the issue is that there can be multiple graph nodes pointing to the same node - for example graph nodes N1 and N2 BOTH can be pointing to N3 - and sometimes N1 would want to mutate N3 and sometimes N2 would want to mutate N3. So a Box wont work, at which point I would need an Rc<Box> and at which point I lost mutability!

We might be thinking of different talks, because this one seems pretty relevant to me. The point is not "RefCell is bad, use vectors" -- the point is to give some thought to software architecture and gauge what guarantees you actually need.

Here's a quote from kyren's blog that I think establishes the connection pretty well:

I’ve gotten a lot of questions over the past year or so that more or less ask:

How do you make a game from scratch in Rust? No, seriously… HOW. I mean, I can see how you can do it in theory , but for some reason when I try to apply patterns that I’m used to from other languages I just hit lots of problems? I’ve heard that this is called “fighting the borrow checker”, but… that doesn’t really seem to help me much. What am I doing wrong?

Or, maybe it’s something like:

I can see how rust is great if you like very strict control, I can see it being used for small utilities or things where security is paramount, but it seems very restrictive! I don’t really see how you could make something large like a game without running into these limitations. How do you structure something like a game without needing Rc and Arc everywhere?

These (straw man) questions are of course about games, but they also mirror sentiments I’ve seen about Rust in general.

2 Likes

I must apologize. I basically read this:

and then didn't read one sentence further

and instead just wrote my comment. My bad, really.

4 Likes

Leaving interior mutability aside for a moment (I explained above why the Box wouldnt work btw, pls correct me if I am wrong), another question - lets say I have a data structure with nodes referring to other nodes etc.. - no interior mutability, just immutable references. So in this case what does Rust recommend - should I use a pointer & reference (and deal with all the lifetime declarations and all that) or use an Rc ?

From my (kind of limited) understanding so far, I see that references and lifetimes make sense when you are dealing with a function call chain where ppl are passing around references here and there. But if we are talking about a data structure thats gonna outlive a function call chain and be around for a long time, an Rc<> makes more sense than an & reference. Again pls let me know what the Rust recommendation is in this case

In one sense I agree, but in another sense I disagree. Arc<Mutex<...>> can serve a different function than Rc<RefCell<...>>, which is to enable communication between threads. And communication between threads is valuable.

2 Likes

This is the main problem with using toy examples to illustrate architecture problems -- a toy example can't capture all the requirements of your real-world problem. It's the constraints of the real world that will determine what architecture you should use. General design principles like "avoid shared mutability" must take a back seat.

It's not clear to me whether this example is meant to be a trimmed-down version of a real problem, or whether it's something you cooked up specifically to illustrate use of RefCell. Of course you can come up with examples where RefCell is the exact right solution; that's why it exists, after all. But there might be just as many examples of problems where RefCell solves the immediate problem, but suboptimally, and it would be better to restructure the data. This is the kind of thing where you can't always tell just from looking at a trimmed-down example because the part that needs to be restructured is probably in the part you trimmed out.

The issue with this question is that there's no one answer -- the best solution for a real-world problem depends on its real-world constraints. So, given sharing but no mutability, you could use Rc, or you could use a vector with indices, or an arena with references. You could even use raw pointers! Rust doesn't recommend anything, it allows you to do whatever is most appropriate for your problem. What's that? Well, the problem statement is far too broad to make a blanket recommendation.

Personally, I think we (the Rust community) sometimes put too much emphasis on being "idiomatic", occasionally even replacing good, performant, obvious code with tricky, slow, buggy code, because it uses more iterators or whatever. Sometimes this happens because we make a recommendation based on local reasoning, but there are nonlocal effects we don't know about. You have to judge whether the advice you get on a forum like this is good or not. We can make suggestions, explore alternatives, and pronounce general principles, but at the end of the day you have to decide what works best in your project. Sometimes what works best is RefCell, and if so, there's no reason to avoid using it.

7 Likes

Thx for the detailed response. The toy example of two nodes N1 and N2 both referring to N3 mutably, is trimmed down from a real use case, and I think its quite a common use case in any large piece of system software.

As a general feedback on documentation: Rust has awesome documentation, thanks to which there is no frustration about lack of information! What I would love to see as an addition to it or an alternate way of documentation is describing real world architectural issues people face in C/C++ and giving example of how that will be done in Rust. So I have been a systems programmer for 20 yrs now working on C and extensively dealing with every single kind of issue that rust is trying to address. So the problems itself are familiar to me - so for me, if I get a "architecture using rust" kind of a cookbook describing real world architectures, that would have really helped me speed up my learning curve in understanding what each concept is really meant to do. The current documentations go at it from the other way round - it talks theory and concepts first like mutability, shared mutability, aliasing etc.. and then shows what to do in rust for each of those. So for me it took me a while to prepare a mental map that "ok this particular thing called mutable xyz is actually something to be used in this kind of architecture that I am familiar with". I wanted a reverse documentation, where we talk about the big problems, show the rust-way of solving it, and that automatically teaches me all these basic concepts. Anyways, once I get more familiar with rust and have more code under my belt, ill pbbly try writing some documentation myself like that

8 Likes

A suggested initial outline for your "reverse documentation" cookbook might get other Rustaceans to start contributing. I suspect it wouldn't take long to assemble a sizable corpus of such examples, at least at the level of textual descriptions. (Adding actual correct code to the examples will tend to take somewhat longer.)

The biggest obstacle I foresee is that real-world problems often are complex, whereas these cookbook snippets inherently will tend to be relatively simple, focussing on only a very few issues per example.

3 Likes

Yep, requests for a "Rust for C++ devs" or a "design patterns cookbook" have come up several times.

https://github.com/nrc/r4cppp is the most complete project I'm aware of in this space. We could probably benefit from a more attempts to tackle it from different angles.

4 Likes

I did not know about that, thanks for the pointer. Let me read through it and see what else can be added there

Probably you're reading too much into that. The sentence above ^^ doesn't mean "bad".

Sometimes one needs a RefCell, it does exist for a reason. But when learning Rust, beginners often reach for tools to imitate patterns in other languages (e.g. interior mutability) because they don't yet understand how to write something in a nicer, more idiomatic, less work-around-ish way. That's why the docs of last-resort tools need to say that "this is a last resort tool, use it only if you are sure you need it".

It has to be communicated somehow.

4 Likes

The fundamental problem with this code is the use of reference counting, which causes you to need interior mutability, which causes you to need runtime checks, and opens up the possibility of runtime panics or deadlocks that we wish were caught at compile time.

If you were to forgo reference counting you could implement your graph without fewer and less risky runtime checks and more bugs caught at compile time. In this case you have nodes that are owned the graph, but there compiler doesn't know that, and that is why you need interior mutability.

To avoid interior mutability, you need to have a single owner of the nodes, something like:

struct Graph {
  all_nodes: Vec<Node>, // or Vec<Box<Node>>
  root: usize, // or root is always 0
}

struct Node {
  children: HashSet<usize>, // or tinyset::SetUsize
}

You can expose the same API, but now the only runtime checks are that your indices are within the array bounds. And those checks will only fail if you use an index that didn't come from the array.

5 Likes