Problems with modifying hash map in a trait

I have a structure I'm planning to hand out in function calls instead a OnceCell as a global variable. I have a hash map in the structure which contains information needed by functions that get called. The trait contains functions to add to the hash map and convert the contents to a vector. I decided to get the information as a vector as I was unable to iterate the hash map. I found example of doing this but none worked. So I am trying to get the following code to work but it looks like having the information in the trait results in borrow problems. Can anyone suggest what I am doing wrong or it there a better way.

Here is the test code I am playing with -

use std::collections::HashMap;

#[derive(Debug)]
struct A {
    aa: String,
}

struct Mapper {
    map: HashMap<String, A>,
}
impl Mapper {
    fn new() -> Mapper {
        Mapper { map: HashMap::new(), }
    }
    fn convert(&self) -> Vec<&A> {
        let mut vec: Vec<&A> =  Vec::from_iter(self.map.values());
        vec
    }
    fn insert(mut self, name: String, item: A) {
        self.map.insert(name, item);
    }
}

fn main() {
    let mapper= Mapper::new();
    let myaa = A { aa: "aa".to_string() };
    let keya = "aa".to_string();
    mapper.insert(keya, myaa); 
    let x = mapper.convert();
    println!("{:?}", x);
}

fn convert(mapp: &HashMap<String, A>) -> Vec<&A> {
    let vec3 = Vec::from_iter(mapp.values());
    return vec3;
}

mut self takes ownership of the struct, moving it. You need a mutable reference instead(&mut self):

use std::collections::HashMap;

#[derive(Debug)]
struct A {
    aa: String,
}

struct Mapper {
    map: HashMap<String, A>,
}
impl Mapper {
    fn new() -> Mapper {
        Mapper { map: HashMap::new(), }
    }
    fn convert(&self) -> Vec<&A> {
        let mut vec: Vec<&A> =  Vec::from_iter(self.map.values());
        vec
    }
    fn insert(&mut self, name: String, item: A) {
        self.map.insert(name, item);
    }
}

fn main() {
    let mut mapper= Mapper::new();
    let myaa = A { aa: "aa".to_string() };
    let keya = "aa".to_string();
    mapper.insert(keya, myaa); 
    let x = mapper.convert();
    println!("{:?}", x);
}

fn convert(mapp: &HashMap<String, A>) -> Vec<&A> {
    let vec3 = Vec::from_iter(mapp.values());
    return vec3;
}
1 Like

Thank you for the suggestion. It worked in my original code.

I have revised my code to reveal the actual error I am seeing. The error I am getting is 'returns a value referencing data owned by the current function' I read that data within the trait is static. What is normally and rust legal to deal with this restriction, if any. Here is my refactored code that demonstrates the issue -

use std::collections::HashMap;
use std::sync::{Mutex};
use std::vec::IntoIter;

#[derive(Debug)]
struct A {
    aa: String,
}

pub struct MapTable(Mutex<HashMap<String, A>>);

impl MapTable {
    pub fn new() -> MapTable {
        MapTable(Mutex::new(HashMap::new()))
    }

    pub fn get_vector<'a>(&self) -> IntoIter<&'a A> {
        let gt=
             self.0.lock()
            .unwrap();
        let vec = Vec::from_iter(gt.values());
        vec.into_iter()
    }
}

fn main() {
    let mut map= MapTable::new();
    let myaa = A { aa: "aa".to_string() };
    let keya = "aa".to_string();
    map.0.lock().unwrap().insert(keya, myaa);
    let x = map.get_vector();
    println!("{:?}", x);
}

For the record, you're not using a trait here; you just have a normal struct.

Ok. thank you for the clarification. A struct with an impl.

You give up access to the HashMap when you drop the MutexGuard you get from .lock().unwrap(). And to access the contents of the HashMap -- like to create the iterator -- you have to borrow the MutexGuard. But you can't move (e.g. return) things that are borrowed.

So it's impossible to create an borrowing iterator of the HashMap in the function and return it, and it's impossible to collect a bunch of references to the contents of the HashMap and return them too.

Here's one way forward, where you return something than can be turned into an iterator instead. Note the deadlock hazards from keeping the MutexGuard around, though.

1 Like

Another way could be to clone/refcount your A's, depending on your real A type and performance requirements.

The whole problem comes about because you're returning references to the contents of the hashmap.

If you return clones of the contents instead, you can unlock the mutex before returning:

use std::collections::HashMap;
use std::sync::{Mutex};

#[derive(Debug, Clone)]
struct A {
    aa: String,
}

pub struct MapTable(Mutex<HashMap<String, A>>);

impl MapTable {
    pub fn new() -> MapTable {
        MapTable(Mutex::new(HashMap::new()))
    }

    pub fn get_vector(&self) -> Vec<A> {
        let gt=
             self.0.lock()
            .unwrap();
            
        gt.values().cloned().collect()
    }
}

fn main() {
    let mut map= MapTable::new();
    let myaa = A { aa: "aa".to_string() };
    let keya = "aa".to_string();
    map.0.lock().unwrap().insert(keya, myaa);
    let x = map.get_vector();
    println!("{:?}", x);
}

playground

Of course, cloning can be an expensive operation, so you may consider storing A's in Arc's, which you can cheaply clone, but it involves an allocation and one more indirection, so it also has some overhead:

use std::sync::Arc;
use std::collections::HashMap;
use std::sync::{Mutex};

#[derive(Debug)]
struct A {
    aa: String,
}

pub struct MapTable(Mutex<HashMap<String, Arc<A>>>);

impl MapTable {
    pub fn new() -> MapTable {
        MapTable(Mutex::new(HashMap::new()))
    }

    pub fn get_vector(&self) -> Vec<Arc<A>> {
        let gt=
             self.0.lock()
            .unwrap();
            
        gt.values().cloned().collect()
    }
}

fn main() {
    let mut map= MapTable::new();
    let myaa = A { aa: "aa".to_string() };
    let keya = "aa".to_string();
    map.0.lock().unwrap().insert(keya, Arc::new(myaa));
    let x = map.get_vector();
    println!("{:?}", x);
}

playground

Thanks for insight into the solution. I want to understand it better. I'm not that concerned about the overhead of cloning. The code will not be used often so there is a negligible performance concern. It seems the collect gets around returning A after the function returns. Both examples work with or without the atomic reference so I don't see it as a factor except Arc makes the result immutable.

I applied your suggestion to my code and it does not build. I think in my case gt becomes a reference as collect cannot be resolved. Comparing the code it looks exactly the same so I don't know where the reference comes from.

The error I am seeing is

Trait FromIterator<Arc<Group, Global>> is not implemented for `Vec

Where Group is a structure equivalent to A in my example.

Please show the code and the full error.

The error is indicated on 'collect'
A value of type Vec<group::group::Group> cannot be built from an iterator over elements of type std::sync::Arc<group::group::Group>

So it sounds like I have to write an iterator trait for GroupTable.

use crate::group::group::Group;
use std::collections::HashMap;
use std::sync::{Arc, Mutex};

pub struct GroupTable(Mutex<HashMap<Arc<String>, Arc<Group>>>);

impl GroupTable {
    pub fn new() -> GroupTable {
        GroupTable(Mutex::new(HashMap::new()))
    }

    pub fn get(&self, name: &Arc<String>) -> 
    
    pub fn get_vector(&self) -> Vec<Group> {
        let gt=
            self.0.lock()
                .unwrap();

        gt.values().cloned().collect()
    }
}

One difference is that you're not returning a Vec<Arc<Group>>, as done in the solution that was posted:

Minor note: by "full error" we mean the entire output from running cargo check in the terminal (not the IDE unless your IDE supports getting the full error), including the notes, hints, etc.

I solved the problem by making the return Vec<Arc>

I'll remember to do the cargo check next time.

1 Like