Search code examples
rustrefcell

Why does RefCell:borrow_mut result in a BorrowMutError when used on both sides of a short-circuiting boolean AND (&&)?


I wrote this code for the leetcode same tree problem:

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

// Definition for a binary tree node.
#[derive(Debug, PartialEq, Eq)]
pub struct TreeNode {
    pub val: i32,
    pub left: Option<Rc<RefCell<TreeNode>>>,
    pub right: Option<Rc<RefCell<TreeNode>>>,
}

impl TreeNode {
    #[inline]
    pub fn new(val: i32) -> Self {
        TreeNode {
            val,
            left: None,
            right: None,
        }
    }
}

struct Solution;

impl Solution {
    pub fn is_same_tree(
        p: Option<Rc<RefCell<TreeNode>>>,
        q: Option<Rc<RefCell<TreeNode>>>,
    ) -> bool {
        match (p, q) {
            (None, None) => true,
            (Some(p), Some(q)) if p.borrow().val == q.borrow().val => {
                // this line won't work, it will cause BorrowMutError at runtime
                // but the `a & b` version works
                return (Self::is_same_tree(
                    p.borrow_mut().left.take(),
                    q.borrow_mut().left.take(),
                )) && (Self::is_same_tree(
                    p.borrow_mut().right.take(),
                    q.borrow_mut().right.take(),
                ));

                let a = Self::is_same_tree(p.borrow_mut().left.take(), q.borrow_mut().left.take());
                let b =
                    Self::is_same_tree(p.borrow_mut().right.take(), q.borrow_mut().right.take());
                a && b
            }
            _ => false,
        }
    }
}

fn main() {
    let p = Some(Rc::new(RefCell::new(TreeNode {
        val: 1,
        left: Some(Rc::new(RefCell::new(TreeNode::new(2)))),
        right: Some(Rc::new(RefCell::new(TreeNode::new(3)))),
    })));
    let q = Some(Rc::new(RefCell::new(TreeNode {
        val: 1,
        left: Some(Rc::new(RefCell::new(TreeNode::new(2)))),
        right: Some(Rc::new(RefCell::new(TreeNode::new(3)))),
    })));

    println!("{:?}", Solution::is_same_tree(p, q));
}

playground

thread 'main' panicked at 'already borrowed: BorrowMutError', src/main.rs:39:23

I think && is a short-circuit operator which means the two expressions won't exist at the same time, so two mutable references should not exist at the same time.


Solution

  • A minimized example:

    use std::cell::RefCell;
    
    fn main() {
        let x = RefCell::new(true);
        *x.borrow_mut() && *x.borrow_mut();
    }
    
    thread 'main' panicked at 'already borrowed: BorrowMutError', src/main.rs:8:27
    

    When you call RefCell::borrow_mut, a temporary value of type RefMut is returned. From the reference:

    The drop scope of the temporary is usually the end of the enclosing statement.

    And in expanded detail:

    Apart from lifetime extension, the temporary scope of an expression is the smallest scope that contains the expression and is for one of the following:

    • The entire function body.
    • A statement.
    • The body of a if, while or loop expression.
    • The else block of an if expression.
    • The condition expression of an if or while expression, or a match guard.
    • The expression for a match arm.
    • The second operand of a lazy boolean expression.

    The failing code would look something like this if it were expanded:

    {
        let t1 = x.borrow_mut();
    
        *t1 && {
            let t2 = x.borrow_mut();
            *t2
        }
    }
    

    This shows how the double borrow occurs.

    As you've already noticed, you can work around this by declaring a variable ahead-of-time. This preserves the short-circuit nature:

    let a = *x.borrow_mut();
    a && *x.borrow_mut();