How to return a reference to a value from Hashmap wrappered in Arc and Mutex in Rust?

Viewed 1771

I got some trouble to return the reference of the value in a HashMap<String,String> which is wrappered by Arc and Mutex for sharing between threads. The code is like this:


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

struct Hey{
    a:Arc<Mutex<HashMap<String, String>>>
}


impl Hey {
    fn get(&self,key:&String)->&String{
        self.a.lock().unwrap().get(key).unwrap()
    }
}

As shown above, the code failed to compile because of returns a value referencing data owned by the current function. I know that lock() returns MutexGuard which is a local variable. But How could I achieve this approach to get a reference to the value in HashMap. If I can't, what is the motivation of Rust to forbidden this?

3 Answers

Let me explain why rustc thinks that your code is wrong.

  1. You can interact with value protected by Mutex only when you have lock on it.
  2. Lock handled by RAII guard.

So, I desugar your code:

fn get(&self,key:&String)->&String{
   let lock = self.a.lock().unwrap();
   let reference = lock.get(key).unwrap();
   drop(lock); // release your lock
   // We return reference to data which doesn't protected by Mutex!
   // Someone can delete item from hashmap and you would read deleted data
   // Use-After-Free is UB so rustc forbid that
   return reference;
}

Probably you need to use Arcs as values:

#[derive(Default)]
struct Hey{
    a:Arc<RwLock<HashMap<String, Arc<String>>>>
}
fn get(&self,key:&String)->Arc<String>{
    self.a.lock().unwrap().get(key).unwrap().clone()
}

P.S. Also, you can use Arc<str> (and I would recommend that), which would save you from extra pointer indirection. It can be built from String: let arc: Arc<str> = my_string.into(); or Arc::from(my_string)

TLDR;

Since, you made the design decision of wrapping your data i.e. HashMap<String, String> in Arc<Mutex<..>> I am assuming you need to share this data across threads/tasks in a thread safe manner. That is the primary use case for this design choice.

So, my suggestion for anyone reading this today isn't a direct answer(returning reference) to this question rather to change the design such that you return an owned data using something like .to_owned() method on the result from the get function call.

fn get(&self, key: &String) -> String {
    let lock = self.a.lock().unwrap(); // #1 Returns MutexGuard
    let val = lock.get(key).unwrap();  
    val.to_owned()
}

Long Form

In the original code snipped, there are actually 2 problems at hand, though only 1 is mentioned in the question.

  1. cannot return value referencing temporary value
  2. returns a value referencing data owned by the current function

Let's try to dig deeper into each of these one by one.

Problem 1

cannot return value referencing temporary value

The temporary value here is referring to MutexGuard. The lock method doesn't return us the reference to the HashMap rather a MutexGuard wrapped around the HashMap. The reason why .get() works on MutexGuard is because it implements DeRef::deref trait. Essentially, it means that MutexGuard can deref into the value it wraps when needed. This deref happens when we call the .get() method.

We can understand the temporary nature of the mutexguard by diving deeper into how the deref method is implemented under the hood.

fn deref<'a>(&'a self) -> &'a T

This means that MutexGuard can only return reference to HashMap for as long as it is alive. Notice the elided lifetime 'a. But since, we don't store the MutexGuard into any local variable rather directly dereference it the rust compiler thinks that it gets dropped right after the get call. The lifetime of the HashMap will be same as MutexGuard. Any result will share the lifetime of the HashMap. Hence, the value/result from .get() method gets dropped instantly.

Solution 1: Store the MutexGuard locally

If we store the mutexguard in a local variable using the let binding. Then the HashMap also has the lifetime of the function scope and the reference/result also has the same lifetime.

let lock = self.a.lock().unwrap(); // storing MutexGuard in local binding
let val = lock.get(key).unwrap(); // val can live as long as lock is alive which is function's lifetime

With this issue fixed, there is just one problem left.

Problem 2

returns a value referencing data owned by the current function

Since, we take the lock in the current function scope, the reference returned from the get function will only be alive for as long as the lock is alive. When we return the reference from the function the compiler will start screaming back at us with the error of data ownership is only valid in the current function scope. It makes sense also, since we only asked(indirectly) for the lock to be active in this function scope. It is semantically wrong to expect the reference to be valid outside the scope of this function.

Solution 2: Change in approach

The whole idea of using Arc and Mutex is to add the capability to update the data between multiple threads safely. This thread safety is provided by the Mutex which enables locking mechanism on the wrapped data, in your case HashMap.

As pointed out by @Abhijit-K, It's not a good design to take the reference of any value outside the scope of the lock. As explained very nicely in the post by @Angelico the lock is dropped within the scope of the function.

Case 1: modifying wrapped data

Only fetch the wrapped value where you have to make changes to the data. Basically, take the lock where you want to change the data, do it in the same scope.

Basically, you pass around the cloned Arc between functions to start with. That is the power of Arc, it can give you many cloned references pointing to the same data on the heap.

Case 2: reading wrapped data

Take a cloned value instead of the reference. Change the approach to return String from &String.

You need to clone the ARC and move the clone ARC to another thread/task. From the clone you can lock and access it. I suggest use RwLock instead of Mutex if there are more accesses than writes.

When you clone ARC you are not cloning the underlying object just the ARC. Also in your case you need to wrap the struct into ARC or change the design, as it is ARC that should be cloned and moved


Approach to share the object should be via guard I believe. With RWLock multiple can read map via the guards:

use async_std::task;
use std::sync::{Arc,RwLock, RwLockReadGuard, RwLockWriteGuard};
use std::collections::HashMap;

#[derive(Default)]
struct Hey{
    a:Arc<RwLock<HashMap<String, String>>>
}

impl Hey {      

    pub fn read(&self) -> RwLockReadGuard<'_, HashMap<String, String>> {
        self.a.read().unwrap()
    }

    
    pub fn write(&self) -> RwLockWriteGuard<'_, HashMap<String, String>> {
        self.a.write().unwrap()
    }    
}


fn main() {
    let h = Hey{..Default::default()};

    h.write().insert("k1".to_string(), "v1".to_string());
   
    println!("{:?}", h.read().get("k1"));
    task::block_on(async move {
        println!("{:?}", h.read().get("k1"));
    });

} 
Related