Dedup logic without getting "cannot return value referencing local variable `queues` returns a value referencing data owned by the current function"

Question

I have the following structs:

use std::sync::Mutex;

pub struct Message {
    id: String,
    content: String,
    last_read: Option<String>,
    uuid: String,
}

impl Message {
    pub fn get_uuid(&self) -> String {
        self.uuid.to_owned()
    }
}

pub struct Queue {
    queue: Vec<Message>, // the actual queue
    read_timeout: u32,   // the amount of time a message is hidden from consumers
    size: u32,           // should always be the same as queue.len()
    uuid: String,
}

pub struct AppState {
    pub queues: Mutex<Vec<Queue>>,
}

Finally, I have some functions that do something with the data. They have duplicate logic, as you can see below. What they do after they get the queue is different, but the way to retrieve the queue is the same.

pub async fn add_message(
    data: AppState
) {
    let queue_uuid = &String::from("my uuid");
    
    let queue_mutex = data.queues.lock();
    let mut queues = match queue_mutex {
        Ok(q) => q,
        Err(_) => return,
    };
    // find matching queue 
    let queue = queues.iter_mut().find(|q| q.uuid == *queue_uuid);
}

pub async fn get_message(
    data: AppState
) {
    let queue_uuid = &String::from("my uuid");
    
    let queue_mutex = data.queues.lock();
    let mut queues = match queue_mutex {
        Ok(q) => q,
        Err(_) => return,
    };
    // find matching queue 
    let queue = queues.iter_mut().find(|q| q.uuid == *queue_uuid);
    // once the queue is found, do some stuff with it 
}

I would like to dedup this logic.

impl AppState {
    pub fn get_queues(&self) -> &Mutex<Vec<Queue>> {
        &self.queues
    }

    pub fn get_queue_by_id(&mut self, uuid: String) -> Result<&mut Queue, String> {
        let queue_mutex = self.queues.lock();
        let mut queues = match queue_mutex {
            Ok(q) => q,
            Err(_) => return Err(String::from("Whoops! Something went wrong")),
        };
        match queues.iter_mut().find(|q| q.uuid == uuid) {
            None => Err(format!("Could not find queue with uuid {}", uuid)),
            Some(q) => Ok(q),
        }
    }
}

However, this will yield an error because I cannot return q as it will be dropped when the function returns. Is there a good solution to this?

P.S: I know that a lot of this could be more efficient - i.e. using a hash map instead of vector to store the current queues. I am a Rust newbie and intend on refactoring a lot of this, so any feedback is helpful! Thanks!

Note: in async contexts blocking like `std::sync::Mutex::lock` requires is not the best idea, use an asynchronous variant like `futures::lock::Mutex` instead. — cafce25, May 21 '23 at 16:32

score 1 · Accepted Answer · answered May 21 '23 at 19:07

In addition to being more appropriate in async contexts than std::sync::Mutex anyways the futures variant has another benefit, it allows your usage pattern by having a MutexGuard::map which let's you return a MappedMutexGuard<Vec<Queue>, Queue>:

impl AppState {
    pub async fn get_queue_by_id(
        &mut self,
        uuid: String,
    ) -> Result<MappedMutexGuard<Vec<Queue>, Queue>, String> {
        let mut queues = self.queues.lock().await;
        match queues.iter_mut().position(|q| q.uuid == uuid) {
            None => Err(format!("Could not find queue with uuid {}", uuid)),
            Some(p) => Ok(MutexGuard::map(queues, |q| &mut q[p])),
        }
    }
}

As far as I know the same is not (yet?) possible with the Mutex in std, see this Rust internals discussion.

score 1 · Answer 2 · answered May 21 '23 at 19:21

If you want to return just a mutex's contents, you can return impl DerefMut<Target = _>. This will allow you to swap out the synchronization wrapper you use without changing the function's signature.

pub fn queues(&self) -> Result<impl Deref<Target = Vec<Queue>> + '_, String> {
    self.queues.lock().map_err(|_| "Mutex poisoned".to_string())
}

// The same since `Mutex` doesn't differentiate between readers and writers. However,
// if a `RwLock` is used instead, this would be different from `queues`.
pub fn queues_mut(&self) -> Result<impl DerefMut<Target = Vec<Queue>> + '_, String> {
    self.queues.lock().map_err(|_| "Mutex poisoned".to_string())
}

This doesn't work for getting a specific queue, but the other answer explains how you can do this with the future crate's mutex.

One way to do this with the std mutex is to take a closure that modifies the selected queue.

pub fn modify_queue_by_id<F, R>(&mut self, uuid: String, f: F) -> Result<R, String>
where
    F: FnOnce(&mut Queue) -> R,
{
    let mut queues = self
        .queues
        .lock()
        .map_err(|_| "Whoops! Something went wrong".to_string())?;

    let queue = queues
        .iter_mut()
        .find(|q| q.uuid == uuid)
        .ok_or_else(|| format!("Could not find queue with uuid {}", uuid))?;

    Ok(f(queue))
}

Since this takes a regular FnOnce closure and not a Future, this ensures that the lock isn't held across an await, even in a single-threaded runtime context.

Dedup logic without getting "cannot return value referencing local variable `queues` returns a value referencing data owned by the current function"

2 Answers2