Returning MutexGuard from inside RwLockReadGuard

help
1

I am trying to figure out how to separate the way I protect my HashMap in front of concurrent updates (inserting or removing values), from the possible read & writes that may happen to its values.

To do so, I thought the proper way would be to wrap a HashMap of Arc inside a RwLock, like the following:

struct Repo(RwLock<HashMap<String, Arc<Mutex<String>>>>);

The problem arises when I want to implement a method for that struct that, given a key, returns the MutexGuard of the corresponding value, if any. See this in the Rust playground.

As you may notice, the compiler complains about I am returning a value referencing data owned by the current function. But I do not really understand why.

If I remove the RwLock wraping the HashMap, like in this example, the compiler does not complain at all, and the method is actually behaving as expected.

How is that? Am I missing something in the code? Or which is the difference between both examples that leads the first one to fail?

2

Accessing the value must go through (i.e., borrow from) the lock guard, otherwise it would lose its entire purpose and be unsound. You can't just return a raw reference to the inner value, because the lock wouldn't then know when to block concurrent access. Thus, you must keep the read lock alive as long as you want to do anything with the values.

You don't.

3

If you switch to the parking_lot locks, you can enable the arc_lock feature to get access to lock guards that don’t use lifetimes, which will let you release the outer lock while still holding the inner one. You may need to be extra careful about avoiding deadlocks, though.

2 Likes
4

Inserting/removing from the map may invalidate pointers to every element in the map, so you can't do this with the struct you currently have. You'll need to separate the ownership of your values from the entries in the map, e.g. by wrapping them in Arcs and then returning clones of them.

1 Like
5

Isn't that what I am doing here? Every value in the map is an Arc<Mutex>.

1 Like
6

Here it is where I get lost. If we where talking about references I would totally agree with you. But here I am cloning an Arc. How is that cloned value aware of the RwLockReadGuard's lifetime? Shouldn't them be independent?

7

It isn't. But the MutexGuard holds a reference to the temporarily-cloned Arc, which gets dropped before get() returns. There's no way in std to return the guard directly when the lock is in a local variable like this, but you can return the cloned Arc<Mutex<...>> for the caller to lock it themselves:

    fn get(&self, id: String) -> Option<Arc<Mutex<String>>> {
        Some(self.0.read().unwrap().get(&id)?.clone())
    }

Alternatively, you could accept a closure to be run on the locked value:

    fn with<T>(&self, id: String, f:impl FnOnce(&mut String)->T) -> Option<T> {
        let inner = self.0.read().unwrap().get(&id)?.clone();
        let mut val = inner.lock().unwrap();
        Some(f(&mut val))
    }

Both of these snippets will release the outer lock before obtaining the inner one, which will allow other code to mess with the structure of Repo concurrently to the inner value being updated. This leads to some classic concurrency hazards like race conditions. For example, something like this could happen:

  • Thread A clones the inner lock
  • Thread B stores a brand-new lock in the same key, replacing the one A has a handle to
  • Thread A updates the internal value, oblivious to the fact that the update will never be seen
3 Likes
8

I was not aware that the MutexGuard is holding a reference of the Arc wrapping the mutex itself. It makes all the sense, since the Arc is in charge of ensuring the Mutex is not dropped while still having references. And the MutexGuard has one. It would be astonishing if the MutexGuard were able to take the ownership of the Arc.

This is the alternative I was thinking about. It is just a little unsatisfying how the code will end up when multiple resources are required (closures inside of closures...). But it's OK, I can handle that :melting_face:.

9

Note that's exactly what parking_lot's arc_lock feature does (which I mentioned earlier).

1 Like
10

I see! I'm taking a look at it.

11

It isn't. It's holding a reference to the Mutex. The Mutex doesn't know when it's inside an Arc or not. The problem is that the reference to that Mutex leads back to a borrow of something which gets dropped in the get method.

I walk through all the borrows in slow motion below. Hopefully it helps.

In what I think you meant to do (get2), the borrow error is because an Arc which is going to drop is still borrowed, and that borrow was used to obtain the MutexGuard<'_, _>. But the MutexGuard does not contain a reference to the Arc itself.

The parking_lot Mutex is not a std Mutex, but it can't intrinsically "know" when it's "in an Arc" either. Compare these locking signatures:

// `std`
impl<T> Mutex<T> {
    // No `Arc` in sight
    pub fn lock(&self) -> LockResult<MutexGuard<'_, T>>

// `parking_lot`
impl<R: RawMutex, T: ?Sized> Mutex<R, T> {
    // Operates on a containing `Arc`
    pub fn lock_arc(self: &Arc<Self>) -> ArcMutexGuard<R, T>
2 Likes
12

@quinedot thank you very much for your answer. get2 is exactly what I meant to do. The .clone() call in the Option<&Arc<_>> is actually a typo, I meant to invoke .cloned() which is practically what you have done in your example. Extremely useful and interesting! Now I see the reference I was missing.