Closure requires unique access to `self` but it is already borrowed

[selvavm]

For past 1 week I am learning Rust. Always, when I think it should be easy, Rust makes it

Below is my code snippet. I am getting the error at k,v

struct MyStruct {
    pub hashmap1: HashMap<String, f32>,
    pub hashmap2: HashMap<String, f32>
}

impl MyStruct {    
    pub fn add(&self) {
        self.hashmap1.par_iter().map(|(k, v)| {
            self.hashmap2.insert(k.to_string(), v+1);
}

[Hyeonu]

I'd highly recommend for everyone to read the chapter 4 of "the book" before writing any Rust code. Other chapters are awesome, but the ch.4 is must read.

https://doc.rust-lang.org/stable/book/ch04-00-understanding-ownership.html

Short answer for your question is that the &T is a shared borrow of the type T, and the &mut T is a unique borrow of the type T. For longer answer, please read the ch.4 first!

[selvavm]

Okay. I will take a rule. Thanks.

Any idea how I can fix this? I would like to read through one self variable (hashmap1) and populate other self variable (hasmap2)

[Hyeonu]

There're several reasons why it's not trivial to fix this simple code. One of them is, the HashMap cannot be mutated concurrently.

It is a common decision between many languages since the thread safe hashmap is a lot slower than the non-thread-safe one, and in the vast majority of use cases the maps are already uniquely accessed by a certain thread on modification. Only difference between the Rust and other languages is, in Rust you'll get compile error instead of some weird and inconsistent runtime behavior.

[L.F]

You need to change fn add(&self) to fn add(&mut self).

[17cupsofcoffee]

There's several issues with your code:

• As mentioned previously, you cannot mutate self inside a method that takes &self - you'll need to change it to &mut self.
• Calling map on an iterator by itself doesn't do anything, as Rust's iterators are lazy. You need to call a method that actually drives the iterator in order for it to actually run. In this case, you're not actually mapping the contents of hashmap1, you just want to run the closure for its side effects, so you should replace map with for_each, which does drive the iterator to completion.
• If you use self directly inside a closure, it will be captured by that closure, preventing you from mutating any of the other fields of self elsewhere. You can force the closure to only capture the hashmap2 field by adding a variable binding and then using that in the closure:

let h2 = &mut self.hashmap2;

self.hashmap1.par_iter().map(|(k, v)| {
    h2.insert(k.to_string(), *v + 1.0);
});

• However, this exposes another issue with your code - you cannot mutate a reference to h2 from inside a parallel iterator! Each time your closure gets called, it could be on a seperate thread, so there's no guarentee that multiple threads wouldn't try to mutate h2 at the same time. As @Hyeonu suggested, you'd need to use some sort of thread-safe hash map to be able to do that, or wrap it in a Mutex or something like that. Or, you could just not use a parallel iterator.

Here's a non-parallel version of the code that works:

use std::collections::HashMap;

struct MyStruct {
    pub hashmap1: HashMap<String, f32>,
    pub hashmap2: HashMap<String, f32>,
}

impl MyStruct {
    pub fn add(&mut self) {
        let h2 = &mut self.hashmap2;

        self.hashmap1.iter().for_each(|(k, v)| {
            h2.insert(k.to_string(), *v + 1.0);
        });
    }
}

And here's a parallel version of the code that works - note that it uses locking to make the mutation of h2 thread safe; using a concurrent data structure would likely be more effecient:

use rayon::iter::IntoParallelRefIterator;
use rayon::iter::ParallelIterator;
use std::collections::HashMap;
use std::sync::Mutex;

struct MyStruct {
    pub hashmap1: HashMap<String, f32>,
    pub hashmap2: Mutex<HashMap<String, f32>>,
}

impl MyStruct {
    pub fn add(&mut self) {
        // You don't need a mutable reference to mutate something
        // that's inside a Mutex - the locking enforces Rust's
        // aliasing rules at runtime.
        let h2 = &self.hashmap2;

        self.hashmap1.par_iter().for_each(|(k, v)| {
            // This will block other threads from using `hashmap2`
            // until the closure returns and the lock is dropped.
            let mut lock = h2.lock().unwrap();
            
            lock.insert(k.to_string(), *v + 1.0);
        });
    }
}

[selvavm]

Thanks it worked like charm. I am a rookie, so please forgive it is basic question.