Trouble with digraphs: cannot borrow as mutable

Question

I created a library to deal with digraphs: nodes that link (reference counted) to zero or one other nodes (as in linked lists, but in a digraph a node can be linked to by more than one node).

I am trying to use my library to create a list with a current node:

struct ListWithPointer<'a> {
    pub nodes: DigraphNodeRef<String>,
    pub current_node: Option<&'a mut DigraphNodeRef<String>>,
}

current_node points to a link in the list.

Now I am trying to move current node to the next element of the list (or to the beginning if the list ended):

fn next_node<'a>(this: &'a mut ListWithPointer<'a>) {
    if this.current_node.is_some() {
        this.current_node.iter_mut().for_each(|a| {
            (*a).as_rc_mut().iter_mut()
                .for_each(|rc| this.current_node = Some(&mut Arc::get_mut(rc).unwrap().next));
        });
    } else {
        this.current_node = Some(&mut this.nodes);
    }
}

but whatever I do, it fails with an error like:

error[E0500]: closure requires unique access to `this.current_node` but it is already borrowed
   --> src/lib.rs:150:51
    |
148 |     fn next_node<'a>(this: &'a mut ListWithPointer<'a>) {
    |                  -- lifetime `'a` defined here
149 |         if this.current_node.is_some() {
150 |             this.current_node.iter_mut().for_each(|a| {
    |             ----------------------------          ^^^ closure construction occurs here
    |             |
    |             borrow occurs here
    |             argument requires that `this.current_node` is borrowed for `'a`
151 |                 (*a).as_rc_mut().iter_mut()
152 |                     .for_each(|rc| this.current_node = Some(&mut Arc::get_mut(rc).unwrap().next));
    |                                    ----------------- second borrow occurs due to use of `this.current_node` in closure

Help to rewrite without errors.

Here is the library code:

use std::sync::Arc;

#[derive(Clone)]
pub struct DigraphNode<T> {
    pub next: DigraphNodeRef<T>, // I made it `pub` to be able `item.next.next()` to remove an item from the middle.
    data: T,
}

impl<T> DigraphNode<T> {
    fn new(next: DigraphNodeRef<T>, data: T) -> Self {
        Self { next, data }
    }
}

pub struct DigraphNodeRef<T> {
    rc: Option<Arc<DigraphNode<T>>>,
}

impl<T> DigraphNodeRef<T> {
    pub fn new() -> Self {
        Self {
            rc: None
        }
    }
    pub fn from_node(value: DigraphNode<T>) -> Self {
        Self::from(Some(Arc::new(value)))
    }
    pub fn from(rc: Option<Arc<DigraphNode<T>>>) -> Self {
        Self {
            rc
        }
    }
    pub fn as_rc(&self) -> &Option<Arc<DigraphNode<T>>> {
        &self.rc
    }
    pub fn as_rc_mut(&mut self) -> &mut Option<Arc<DigraphNode<T>>> {
        &mut self.rc
    }
    pub fn is_none(&self) -> bool {
        self.rc.is_none()
    }
    pub fn remove(&mut self) -> bool {
        if let Some(rc) = self.rc.clone() {
            self.rc = rc.next.rc.clone();
            true
        } else {
            false
        }
    }
    pub fn prepend(&mut self, value: T) -> Self {
        let new_node = DigraphNode::new(self.clone(), value);
        let new_node_ref = DigraphNodeRef::from_node(new_node);
        *self = new_node_ref.clone();
        new_node_ref
    }
    pub fn node(&self) -> Option<DigraphNode<T>>
        where T: Clone
    {
        self.rc.clone().map(|node| (*node).clone())
    }
    /// TODO: Should return a reference.
    pub fn data(&self) -> Option<T>
        where T: Clone
    {
        self.rc.clone().map(|node| (*node).data.clone())
    }
    pub fn values(self) -> DigraphNodeValuesIterator<T> {
        DigraphNodeValuesIterator {
            underlying: self.clone()
        }
    }
}

impl<T> Clone for DigraphNodeRef<T> {
    fn clone(&self) -> Self {
        Self { rc: self.rc.clone() }
    }
}

impl<T> Iterator for DigraphNodeRef<T> {
    type Item = Arc<DigraphNode<T>>;

    fn next(&mut self) -> Option<Self::Item> {
        if let Some(rc) = self.rc.clone() {
            self.rc = rc.next.rc.clone();
            Some(rc.clone())
        } else {
            None
        }
    }
}

pub struct DigraphNodeValuesIterator<T> {
    underlying: DigraphNodeRef<T>,
}

impl<T: Clone> Iterator for DigraphNodeValuesIterator<T> {
    type Item = T;

    fn next(&mut self) -> Option<Self::Item> {
        self.underlying.next().map(|node| node.data.clone())
    }
}

Answer 1

In Rust the mutable access is ensured to be exclusive, i.e. if you hold a reference, some other code can't grab a mutable reference.

Problem is this line:

this.current_node.iter_mut().for_each(...)

It grabs a mutable access to current_node, so it can't regain it again down the line. Not to mention that iterating over Option is a strange decision.

If you want to move current_node to a different place, I'd try to reorganize your code such that reads are separate from writes, and they are performed in a sequence, instead of trying to do it in one go:

// detach the current_node for moving
if let Some(current_node_to_move) = this.current_node.take() {   
    let new_current_node_ref: &mut ... = ... // find new location logic
    new_current_node_ref.replace(current_node_to_move);
} else {
    ...
}

Here in line 1 it does a write None update to current_node via this, but immediately relinquishes the mutable reference. Line 2 does a read (search), but also grabs a mutable reference to a new location. Line 3 writes to this location.

To get the linked list implementation right, I recommend https://rust-unofficial.github.io/too-many-lists/