'already borrowed: BorrowMutError' vec of traits

I believe I am running into some sort of interior mutability issue:

Playground BorrowMutError

  thread 'main' panicked at 'already borrowed: BorrowMutError', libcore/result.rs:1009:5
stack backtrace:
  ...
  10: <core::cell::RefCell<T>>::borrow_mut
             at libcore/cell.rs:885
  11: playground::Scheduler::schedule_timing_target
             at src/main.rs:84
  12: <playground::BootScene as playground::NodeTrait>::enter
             at src/main.rs:49
  13: playground::main
             at src/main.rs:127
  ...

Basically I create two Rc-RefCell trait objects and put them into a vec. Everything was working fine until I decided to put the vec into a the Scheduler struct and add methods to mutate it using what I believe is interior mutability.

The problem happens when I try to call borrow_mut() on an element from the iterator. From what I have read about vec iterators is that the iterator "borrows-out" the element which means I can't borrow_mut it myself. This seems like a catch-22. This only shows up when I put the vec collection inside my struct "Scheduler". Some how the extra indirection/level causes a borrow some where that I can't see. I wish the backtrace would show a trace of the borrow history so I could see where the mysterious borrow is occurring.

Any ideas?

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

type RefNode = Rc<RefCell<NodeTrait>>;

trait NodeTrait {
    fn id(&self) -> usize {
        0
    }
    fn set_id(&mut self, id: usize);
    fn paused(&self) -> bool;
    fn pause(&mut self, pause: bool);

    fn enter(&self, sch: &Scheduler);
    fn exit(&self, sch: &Scheduler);
}

// ======================================================
struct BootScene {
    id: usize,
    paused: bool,
}

impl BootScene {
    fn new() -> RefNode {
        Rc::new(RefCell::new(Self {
            id: 0,
            paused: true,
        }))
    }
}

impl NodeTrait for BootScene {
    fn id(&self) -> usize {
        self.id
    }
    fn set_id(&mut self, id: usize) {
        self.id = id;
    }
    fn paused(&self) -> bool {
        self.paused
    }
    fn pause(&mut self, pause: bool) {
        self.paused = pause;
    }

    fn enter(&self, sch: &Scheduler) {
        sch.print();
        sch.schedule_timing_target(self.id());
        println!("---------------------");
        sch.print();
    }

    fn exit(&self, sch: &Scheduler) {
        sch.print();
        sch.unschedule_timing_target(self.id());
        println!("---------------------");
        sch.print();
    }
}

struct Scheduler {
    scenes: Vec<RefNode>,
}

impl Scheduler {
    fn new() -> Self {
        Self { scenes: Vec::new() }
    }

    fn register_timing_target(&mut self, target: RefNode) {
        self.scenes.push(target);
    }

    fn schedule_timing_target(&self, target_id: usize) {
        for target in &self.scenes {
            let mut t = target.borrow_mut();  // <====== BORROW ERROR
            if t.id() == target_id {
                t.pause(false);
                return;
            }
        }
    }

    fn unschedule_timing_target(&self, target_id: usize) {
        for target in &self.scenes {
            if target.borrow().id() == target_id {
                target.borrow_mut().pause(true);
                return;
            }
        }
    }

    fn print(&self) {
        let mut c = 0;
        for target in &self.scenes {
            println!("c: {}", c);
            println!(
                "id: {}, paused: {}",
                target.borrow().id(),
                target.borrow().paused()
            );
            c += 1;
        }
        println!("###########################");
    }
}

fn main() {
    let mut sch = Scheduler::new();

    let s1 = BootScene::new();
    s1.borrow_mut().set_id(1);
    sch.register_timing_target(s1.clone());

    let s2 = BootScene::new();
    s2.borrow_mut().set_id(3);
    sch.register_timing_target(s2.clone());

    s2.borrow().enter(&sch);
    s2.borrow().exit(&sch);
}
1 Like

You've borrowed a RefNode as part of calling enter():

s2.borrow().enter(&sch);

Within enter, you trigger that schedule_timing_target(), which ends up trying to borrow that same s2 mutably, but it's still borrowed immutably due to the enter() line above.

1 Like

Aaahhhh. That makes sense. Hmmm.... now I wonder how I would handle this. It seems natural to want to do this...I think.

Is there a tool that will show a trace/walk of borrows so I can see where borrows are occurring on my behalf? The debugger only show borrow counts but not a trace.

The "trick" with RefCell usage is borrow at an entrypoint to some API/functionality, and then pass the reference around from that point forward, until the API/functionality finishes. So effectively minimize the number of places you borrow/borrow_mut because otherwise you risk losing track of borrow state.

In your example, you can try an approach like this. I'm not sure if it's an artifact of your repro or not, but it doesn't really make sense to iterate to find the RefNode to set its pause state when you already have a handle to it (i.e. s2). Maybe your real case is more elaborate though.

Also, if your types that you want to mutate via interior consist of simple Copy types, such as usize and bool for the id and pause state, respectively, then you can encapsulate those fields in a Cell and drop the RefCell entirely.

There's no tool that I'm aware of. When I've hit these, I just sprinkle in println statements where borrows occur.

1 Like

Thanks +vitalyd,

:wink: tried several things like you mentioned, but you are correct that my 'Gist' is borrowing while trying to call enter() even if I borrow for both enter/exit prior.

I could look into Cell to see if my objects are lite enough to Copy.

Just to put a bit more meat on the example, here is what your original code could look like if you were to use Cell instead.

1 Like

Well...it seems I still have a lot to learn about Rust! I didn't know I could use Cell like that. :thinking:

Now because you use Cell then that means copying is taking place, correct?

Thanks

Cell<T> has a few impl blocks that define its overall API. Some of those impl blocks have a T: Copy bound, e.g. Cell::get() is only callable/available when T: Copy. So yeah, when you use those APIs copying will take place. But if your underlying types are cheap to copy, as the case in your example, then it's totally fine. RefCell is more useful in cases where the type isn't Copy, e.g. String.

In general, consider reaching for RefCell as a checkpoint: this is a good time and place to reassess your design, and see if there's a way to avoid using it, either by using Cell or designing the implementation (and/or API) differently. Occasionally, you'll realize that RefCell is still the best alternative, but other times you'll find a better way.

1 Like

Good suggestions and advice. I will have to take a closer look at Cell and RefCell in relation to my project. My objects aren't that heavy to Cell could be a potential.

Thanks vitalyd.