Best practice to keep abstractions and avoid "cannot borrow as mutable more than once"

help
1

Hello everybody,

this is not a real problem because I know how to write code that works but to do that I have to avoid certain abstraction that I'd like to keep, so I am looking for suggestion about how to do this "the Rust way".

Basically I have an object that encapsulates a cache and a dependency from a library that uses data from that cache. I'd like to keep some data private and expose the functionality via methods but I get into the cannot borrow as mutable more than once. Here is a simplification of the code (DataContext is from the external library):

use std::collections::HashMap;

struct EditContext {
    cache: HashMap<usize, Resource>,
    context: DataContext,
}

struct DataContext {
    data: usize,
}

struct Resource {
    pub data: usize,
}

impl EditContext {
    pub fn new() -> Self {
        EditContext {
            cache: HashMap::new(),
            context: DataContext { data: 0 },
        }
    }

    pub fn get_resource(&mut self, resource_id: usize) -> &Resource {
        if !self.cache.contains_key(&resource_id) {
            self.cache.insert(resource_id, Resource { data: 42 });
        }
        self.cache.get(&resource_id).unwrap()
    }

    pub fn do_edit(&mut self, resource: &Resource) {
        self.context.do_something(resource);
    }
}

impl DataContext {
    pub fn do_something(&mut self, resource: &Resource) {
        self.data = resource.data;
        println!("data: {}", self.data);
    }
}

fn main() {
    let mut edit = EditContext::new();
    let resource_id = 0;
    let resource = edit.get_resource(0);
    // The error happens on the next line.
    edit.do_edit(resource);
}

Calling do_edit is perfectly safe because the only way of invalidating the cache is by calling get but the compiler does not know that. I can make the code working by making some members public and then replacing the code in main direct access:

    if !edit.cache.contains_key(&resource_id) {
        edit.cache.insert(resource_id, Resource { data: 42 });
    }
    let resource = edit.cache.get(&resource_id).unwrap();
    edit.context.do_something(resource);

but it would be much better to be able to call a sequence of methods on edit.

Suggestions? Thanks!

2

One option is to use Arc to return a non-lifetime-encumbered Resource from get_resource:

(untested)

use std::collections::HashMap;
use std::sync::Arc;

struct EditContext {
    cache: HashMap<usize, Arc<Resource>>,
    context: DataContext,
}

impl EditContext {
    pub fn get_resource(&mut self, resource_id: usize) -> Arc<Resource> {
         self.cache
             .entry(resource_id)
             .or_insert_with(|| Arc::new( Resource { data: 42 } ))
             .clone()
     }
}

// NB: Unchanged code has been omitted for brevity
3

I did not think about that. Thank you very much.

4

Another way to think about the problem is by asking the question, "what purpose does splitting up get_resource() and do_edit() serve?"

On the surface, it looks like maybe this makes sense as an abstract API, because getting a resource and editing a resource are two distinct operations. But if you dig further, you recognize that it isn't really useful to call do_edit() without first calling get_resource() (because you need a &Resource to edit). These methods are in fact tightly coupled.

Let us assume that get_resource() on its own is useful, because &Resource is useful in some other context. What you can do with do_edit() is break the coupling by accepting resource_id: usize instead of resource: &Resource. You will also want to rewrite get_resource() to take advantage of disjoint borrows for code reuse:

    fn get_resource_inner(cache: &mut HashMap<usize, Resource>, resource_id: usize) -> &Resource {
        if !cache.contains_key(&resource_id) {
            cache.insert(resource_id, Resource { data: 42 });
        }
        cache.get(&resource_id).unwrap()
    }

    pub fn get_resource(&mut self, resource_id: usize) -> &Resource {
        Self::get_resource_inner(&mut self.cache, resource_id)
    }

    pub fn do_edit(&mut self, resource_id: usize) {
        let resource = Self::get_resource_inner(&mut self.cache, resource_id);
        self.context.do_something(resource);
    }

Now it works with:

fn main() {
    let mut edit = EditContext::new();
    let resource_id = 0;
    edit.do_edit(resource_id);
}

What happened?

First, get_resource() was extracted to an associated function named get_resource_inner(). Instead of taking &mut self, it takes &mut HashMap<usize, Resource>.

Then, both get_resource() and do_edit() call this associated function passing &mut self.cache. This is what makes it work: The compiler has knowledge of the structure fields and it can trivially tell that self.cache and self.context (in the do_edit() method) are disjoint. These can each be borrowed exclusively for their own short lifetimes [1].

We've done a few things, here. We managed to break the tight coupling on the API by not trying to thread a shared borrow out of the struct and back into it. And in turn that simplifies the API (IMHO, anyway) and allows it to "just work".

We had to resort to a private associated function to leverage the compiler's knowledge of disjoint fields and (importantly) to share code between two other methods. It's a bit of a hack, but it's better than "cannot borrow `...` as immutable because it is also borrowed as mutable" when you just want to be productive!

I don't know, you might disagree with the opinion that the coupling between get_resource() and do_edit() should be broken. If you feel strongly about a dissent, then another option to consider is moving the do_edit() method to Resource [2]: Rust Playground

  1. The lifetime of &'a mut self.cache lasts until the last use of the get_resource_inner() return value because the returned shared borrow is rooted in the exclusive borrow. You can see this by trying to borrow self.cache between the get_resource_inner() and do_something() calls: Rust Playground ↩︎

  2. But now that I think about it, this still decouples get_resource() and do_edit()! So maybe this is a good hint that tight coupling (interface segregation principle; the "I" in "SOLID") is fundamental to the problem. ↩︎

3 Likes
5

I think this is a very good suggestion. Obviously the code I posted was a very simple example and the real code is much more complex (with multiple caches, different types of resources and so on) but using a private associated function to let the compiler know two fields are disjointed is something I am not used to and probably solves a whole class of lifetime-related errors. Thank you very much for this suggestion.

1 Like
6

It does! Niko's blog post on the topic is an excellent read: Baby Steps (smallcultfollowing.com) There are some follow ups and other threads on a related subject, "partial borrowing", including many proposals that hope to extend the language as a solution.

Another interesting, if maybe tangential, topic is "splitting borrows", which allows an exclusive reference to be split into multiple disjoint mutable references. It happens to fit the same kind of mutable aliasing problem but deals only with exclusive references. There is a lot to read on both subjects! From official and unofficial sources. (Sorry I can't list them now, short on time.)

Best of luck!

1 Like