How to solve the problem that after getting independent data from a mutex-protected object, the lock must be kept in Rust?

Question

In actix-web route function, I find that after getting independent data from mutex-protected object, the mutex must be in locking for using these data. If the mutex only locks for getting data, there will be a compile error for using data. Here is code can be compiled directly:

use std::collections::HashMap;
use std::fmt::Debug;
use std::sync::Mutex;
use async_trait::async_trait;
use actix_web::{web, App, HttpServer, HttpResponse, Result as ActixResult};

#[async_trait]
pub trait Reader: Send + Debug { }

#[derive(Clone, Debug)]
pub struct RemoteReader;

#[async_trait]
impl Reader for RemoteReader { }

#[derive(Default)]
pub struct DataReaders(HashMap<String, Box<dyn Reader>>);

impl DataReaders {
    pub fn get_readers(&self, ids: &str) -> ActixResult<Vec<&dyn Reader>> {
        let mut readers = Vec::new();
        for id in ids.split(',') {
            readers.push(self.0.get(id).unwrap().as_ref());
        }
        Ok(readers)
    }
}

async fn get(data: web::Data<Mutex<DataReaders>>) -> ActixResult<HttpResponse> {
    let mut readers = data.lock().unwrap().get_readers("1,2")?;
    if readers.is_empty() {
        return Ok(HttpResponse::NotFound().finish())
    }
    // Below will call multiple asynchronous functions, so I do not want to
    // hold the lock of DataReaders for a long time.
    readers.clear();
    Ok(HttpResponse::Ok().finish())
}

#[actix_web::main]
async fn main() -> std::io::Result<()> {
    let mut readers = DataReaders{ 0: Default::default() };
    readers.0.insert("1".to_string(), Box::new(RemoteReader{}));
    readers.0.insert("2".to_string(), Box::new(RemoteReader{}));
    let counter = web::Data::new(Mutex::new(readers));

    HttpServer::new(move || {
        App::new()
            .app_data(counter.clone())
            .route("/", web::get().to(get))
    }).bind(("127.0.0.1", 8080))?.run().await
}

compile this code and get error:

error[E0716]: temporary value dropped while borrowed
  --> src/main.rs:30:23
   |
30 |     let mut readers = data.lock().unwrap().get_readers("1,2")?;
   |                       ^^^^^^^^^^^^^^^^^^^^                    - temporary value is freed at the end of this statement
   |                       |
   |                       creates a temporary value which is freed while still in use
31 |     if readers.is_empty() {
   |        ------- borrow later used here
   |
   = note: consider using a `let` binding to create a longer lived value

I only want to use mutex to protected the HashMap because it will be altered in other route functions. I think the Vec<&dyn Reader> object returned from 'get_readers' should not be protected because all items in it is readonly. So how can I solve this? thanks!

Answer 1

Consider what happens if another route modifies the HashMap in the interim: any &dyn Reader reference that you're holding would be invalidated not only by any modification of the specific map item you're borrowing from, but also by any insertion that causes the map to reallocate (actually any mutation of the HashMap, even of an unrelated item, would invalidate your reference because borrowing occurs at the level of the whole map not individual items).

Therefore unless you move/copy/clone what you're using from the HashMap, the Mutex will need to remain locked. You might therefore consider holding your objects in refcounted allocations such as Arc<dyn Reader> instead of in Boxes, as these smart pointers can be cheaply cloned so that the lock can then immediately be dropped:

use std::collections::HashMap;
use std::fmt::Debug;
use std::sync::{Arc, Mutex};
use async_trait::async_trait;
use actix_web::{web, App, HttpServer, HttpResponse, Result as ActixResult};

#[async_trait]
pub trait Reader: Send + Debug { }

#[derive(Clone, Debug)]
pub struct RemoteReader;

#[async_trait]
impl Reader for RemoteReader { }

#[derive(Default)]
pub struct DataReaders(HashMap<String, Arc<dyn Reader>>);

impl DataReaders {
    pub fn get_readers(&self, ids: &str) -> ActixResult<Vec<Arc<dyn Reader>>> {
        let mut readers = Vec::new();
        for id in ids.split(',') {
            readers.push(self.0.get(id).unwrap().clone());
        }
        Ok(readers)
    }
}

async fn get(data: web::Data<Mutex<DataReaders>>) -> ActixResult<HttpResponse> {
    let mut readers = data.lock().unwrap().get_readers("1,2")?;
    if readers.is_empty() {
        return Ok(HttpResponse::NotFound().finish())
    }
    // Below will call multiple asynchronous functions, so I do not want to
    // hold the lock of DataReaders for a long time.
    readers.clear();
    Ok(HttpResponse::Ok().finish())
}

#[actix_web::main]
async fn main() -> std::io::Result<()> {
    let mut readers = DataReaders{ 0: Default::default() };
    readers.0.insert("1".to_string(), Arc::new(RemoteReader{}));
    readers.0.insert("2".to_string(), Arc::new(RemoteReader{}));
    let counter = web::Data::new(Mutex::new(readers));

    HttpServer::new(move || {
        App::new()
            .app_data(counter.clone())
            .route("/", web::get().to(get))
    }).bind(("127.0.0.1", 8080))?.run().await
}