Rust destructure enum protected by std::sync::RwLock, return with reference

Question

I have a vector of enum elements I'd like to access in a thread-safe manner. I have interfaces that need to degrade the enum values to an option, as to not expose the inner structures to the public API. For that I have enclosed each of the vector elements into an RwLock:

enum Container<T>{
    Empty, Emptied, Full(T)
}

use std::sync::{RwLock, RwLockReadGuard};

struct Ship<T>{
    content: Vec<RwLock<Container<T>>>
}

impl<T> Ship<T>{

    pub fn get_enum(&self, index: usize) -> Option<RwLockReadGuard<'_, Container<T>>>{
       self.content[index].read().ok()
    }

    // The below code fails to compile: 
    // pub fn get(&self, index: usize) -> Option<&T>{
    //   match self.content[index].borrow().ok().unwrap() {
    //       Container::Full(c) => Some(c), 
    //       _=> None
    //   }
    // }
    
    pub fn get_mut(&mut self, index: usize) -> Option<&mut T>{
       match self.content[index].get_mut().ok().unwrap() {
           Container::Full(c) => Some(c), 
           _=> None
       }
    }
}

The above seem to compile, but is it safe code to run? I would also like to provide an interface for read-only access, but I failed to find the corresponding function in the API. For that what can be used?

"An interface for readonly access" without locking would be unsound, as a shared reference to the RwLock (or its container) doesn't guarantee that there are no outstanding writers: the entire point of an RwLock is that you can get a mutable reference to the contents from an immutable reference to the lock. — Masklinn, Aug 21 '23 at 08:20
I would not like to avoid locking it, but I'd like to have different parts of it under different locks so in a way different items could be written independent of each other. — Dávid Tóth, Aug 21 '23 at 08:22
You... do? Just expose a variant of `get_enum` which calls `write`. Also you may want to consider `try_read`/`try_write` as e.g. if there's already a write lock being held on `content[index]` the `get_enum` call will block until that lock is released. That can easily lead to deadlocks if the indexes are not correctly checked and you try to access the same index as both mutable and immutable. — Masklinn, Aug 21 '23 at 08:25
I think I might need to redesign this to be useful and not convoluted at all.. The way I understand from the comments and the answers is that if I want to have thread-safe interfaces I need to expose `RwLock` on them, e.g. to return with `Option>` or `Option<&RwLock>` in the examples case — Dávid Tóth, Aug 21 '23 at 08:29

cafce25 · Answer 1 · 2023-08-21T08:47:31.317

3

It is certainly safe to run, after all Rust guarantees that if you don't use unsafe it's memory and thread safe. But it most certainly doesn't do what you want it to, since get_mut requires you to only have one reference to the RwLock but in that case you could just forgo the RwLock alltogether and have the same effect, but that doesn't let you share a Ship across threads, it requires an exclusive reference.

You likely want to use a RwLock that supports mapping on it's guards such as the one from parking_lot:

use parking_lot::{MappedRwLockWriteGuard, RwLock, RwLockReadGuard, RwLockWriteGuard};

/* ... */

impl<T> Ship<T> {
    pub fn get_enum(&self, index: usize) -> Option<RwLockReadGuard<'_, Container<T>>> {
        self.content.get(index).map(|g| g.read())
    }

    pub fn get_mut(&self, index: usize) -> Option<MappedRwLockWriteGuard<'_, T>> {
        RwLockWriteGuard::try_map(self.content.get(index)?.write(), |c| match c {
            Container::Full(c) => Some(c),
            _ => None,
        })
        .ok()
    }
}

Playground

edited Aug 21 '23 at 08:47

answered Aug 21 '23 at 08:12

cafce25

15,907
4
25
31

sorry I struggle to understand this in its current form.. can you please expand on what do you think I expect it to do and what is the difference between that and what it actually does? And is it possible to do soimething like this for non-mut references? – Dávid Tóth Aug 21 '23 at 08:16
Well it's thread-safe in the sense that it can only work if you have a unique / mut reference to the `Ship`, so either the `Ship` is not shared at all, or already behind some sort of lock e.g. a mutex or an rwlock which guarantees there *can't be* any other outstanding reference to the RwLock and thus no need to lock it. – Masklinn Aug 21 '23 at 08:19
1

@DávidTóth I think you expect to be able to use it in several threads or tasks concurrently, that won't be possible since you need exclusive access to use `get_mut`. – cafce25 Aug 21 '23 at 08:20
so it isn't thread-safe at all if I understand it correctly that is important information! Thanks – Dávid Tóth Aug 21 '23 at 08:21
2

It is thread-safe in the sense that it's free of any sort of data race, because the compiler will prevent you (statically) from calling the method on shared objects. – Masklinn Aug 21 '23 at 08:25
What you wrote is now half what I was looking for, but acquiring a RwLockWriteGuard means that I need to have mutable access to the whole object, right? AFAIU that can only be done if the whole object itself is behind a Mutex or a lock, right? – Dávid Tóth Aug 21 '23 at 08:34
1

No, that's the whole point of interior mutability, a `RwLock` does not require mutable access to it and can still give you mutable access to it's interior, as can be seen by the signature of `Ship::get_mut` it takes `&self`, not `&mut self`! The guard is more or less just a reference that informs the `RwLock` when it goes out of scope. – cafce25 Aug 21 '23 at 08:37
I see `pub fn get_mut(&mut self) -> LockResult<&mut T>` in https://doc.rust-lang.org/std/sync/struct.RwLock.html#method.get_mut; Maybe you are looking at the parking lot docs? – Dávid Tóth Aug 21 '23 at 08:39
1

But the code isn't using `RwLock::get_mut` you named your method the same.. But yea this **is** using the `parking_lot::RwLock` as the one from `std` does not support mapping. – cafce25 Aug 21 '23 at 08:40
@DávidTóth `RwLock::get_mut` takes an `&mut` so it can yield a reference to the interior object without acquiring a lock (because the input `&mut` means there is only one outstanding reference to the lock itself, so there's no concurrency to be had). `write` takes an `&self`, and gives an `&mut T` (ish, really a guards which derefmuts), so it can provide a mutable reference from a shared one. – Masklinn Aug 21 '23 at 10:43

Rust destructure enum protected by std::sync::RwLock, return with reference

1 Answers1