Deref for two nested RwLocks

[romamik]

I am trying to create a simple reactive system in Rust. I ended up with a data structure like this:

struct Context {
    signals: RwLock<HashMap<usize, RwLock<StoredSignal>>>
}
type StoredSignal = Box<dyn Any>
struct Signal {
    cx: &'static Context,
    id: usize
}

This allows me to lock the HashMap for writing only when creating signals, and lock individual signals for writing. And also, this allows to implement Copy for signal which can be handy. However, I have a problem accessing signal data in a convenient way.

I'd like to have a SignalLock struct that implements Deref and can be created from Signal. But because of the two RwLocks, I have to perform two locks, and I end up with self-references if I want my SignalLock to hold both. How can I approach this?

Code in playground: Rust Playground

[jumpnbrownweasel]

    signals: RwLock<HashMap<usize, RwLock<StoredSignal>>>,

I think using this type means there is a design problem.

Since RwLock<StoredSignal> is not stored on the heap separately (not in a Arc), this means you have to lock RwLock<HashMap> while you're accessing the StoredSignal. Otherwise, the HashMap entry could move in memory when another thread modifies the HashMap.

In this case, you don't need to lock the RwLock<StoredSignal>, since it is always protected by the outer RwLock. Removing the inner RwLock would solve the problem you asked about.

But if you intend to access the StoredSignal separately from the HashMap, you'll need to put it in an Arc, or more likely in an Arc<RwLock>. This would allow you to drop the HashMap lock after you obtain the entry, which also solves the problem you asked about. After dropping the HashMap lock, obtain the inner lock. You would only need to store the lock guard for the inner lock in your struct.

---

The choice between these two options is a trade-off:

• With RwLock<HashMap<usize, StoredSignal>> you can only access one StoredSignal at a time for write access.

• With RwLock<HashMap<usize, Arc<RwLock<StoredSignal>>>> you can access multiple StoredSignals concurrently for write access. The added cost is a separate allocation (the Arc) for each StoredSignal.

In both cases you can access multiple StoredSignals concurrently for read access.

[jonh]

https://crates.io/crates/ouroboros

[jumpnbrownweasel]

Hopefully a self-referential struct is not necessary, since Rust does not support self-referential structs.. If it is truly necessary, I think it's important to point out that all of the crates supporting self-referential structs have had multiple soundness problems. Perhaps ouroboros has no soundness problems that are known currently, but there is risk in using any of them.

[romamik]

The idea is that I can have read lock for the map and write lock for the signal. RwLock allows multiple read locks, so I can modify signal without obtaining exclusive lock on the map. Introducing Arc, while it is possible and should solve the issue seems unnecessary, as I actually do not want shared ownership, just temporary lock.

[romamik]

It was possible to make this work with ouroboros:

use ouroboros::self_referencing;
use std::{
    any::Any,
    collections::HashMap,
    marker::PhantomData,
    ops::{Deref, DerefMut},
    sync::{atomic::AtomicUsize, RwLock, RwLockReadGuard, RwLockWriteGuard},
    thread::spawn,
};

#[derive(Default)]
struct Context {
    next_id: AtomicUsize,
    signals: RwLock<HashMap<usize, RwLock<StoredSignal>>>,
}

impl Context {
    pub fn create_signal<T: Any + Send + Sync>(&'static self, data: T) -> Signal<T> {
        let id = self
            .next_id
            .fetch_add(1, std::sync::atomic::Ordering::Relaxed);
        let signal = StoredSignal {
            id,
            data: Box::new(data),
        };
        self.signals
            .write()
            .unwrap()
            .insert(id, RwLock::new(signal));
        Signal {
            cx: self,
            id,
            _ty: PhantomData,
        }
    }
}

struct StoredSignal {
    id: usize,
    data: Box<dyn Any + Send + Sync>,
}

#[derive(Clone, Copy)]
struct Signal<T> {
    cx: &'static Context,
    id: usize,
    _ty: PhantomData<T>,
}

impl<T> Signal<T> {
    pub fn get(&self) -> SignalLock<T> {
        let lock = SignalLockBuilder {
            map_lock: self.cx.signals.read().unwrap(),
            lock_builder: |map_lock| map_lock.get(&self.id).unwrap().read().unwrap(),
            ty: PhantomData,
        }
        .build();
        lock
    }

    pub fn get_mut(&self) -> SignalLockMut<T> {
        SignalLockMut(
            SignalLockMutInnerBuilder {
                map_lock: self.cx.signals.read().unwrap(),
                lock_builder: |map_lock| map_lock.get(&self.id).unwrap().write().unwrap(),
                deref_mut_called: false,
                ty: PhantomData,
            }
            .build(),
        )
    }
}

#[self_referencing]
struct SignalLock<'a, T> {
    map_lock: RwLockReadGuard<'a, HashMap<usize, RwLock<StoredSignal>>>,
    #[borrows(map_lock)]
    #[covariant]
    lock: RwLockReadGuard<'this, StoredSignal>,
    ty: PhantomData<T>,
}

impl<T: 'static> Deref for SignalLock<'_, T> {
    type Target = T;

    fn deref(&self) -> &Self::Target {
        self.borrow_lock().data.downcast_ref().unwrap()
    }
}

#[self_referencing]
struct SignalLockMutInner<'a, T> {
    map_lock: RwLockReadGuard<'a, HashMap<usize, RwLock<StoredSignal>>>,
    #[borrows(map_lock)]
    #[covariant]
    lock: RwLockWriteGuard<'this, StoredSignal>,
    deref_mut_called: bool,
    ty: PhantomData<T>,
}

struct SignalLockMut<'a, T>(SignalLockMutInner<'a, T>);

impl<T: 'static> Deref for SignalLockMut<'_, T> {
    type Target = T;

    fn deref(&self) -> &Self::Target {
        self.0.borrow_lock().data.downcast_ref().unwrap()
    }
}

impl<T: 'static> DerefMut for SignalLockMut<'_, T> {
    fn deref_mut(&mut self) -> &mut Self::Target {
        self.0.with_deref_mut_called_mut(|called| *called = true);
        self.0
            .with_lock_mut(|lock| lock.data.downcast_mut().unwrap())
    }
}

impl<'a, T> Drop for SignalLockMut<'a, T> {
    fn drop(&mut self) {
        if *self.0.borrow_deref_mut_called() {
            println!("DerefMut was called {}", self.0.borrow_lock().id);
        }
    }
}

fn main() {
    let cx = Box::leak(Box::new(Context::default()));
    let signal: Signal<i32> = cx.create_signal(100);
    dbg!(*signal.get());
    dbg!(*signal.get_mut());
    dbg!(*signal.get());
    *signal.get_mut() = 200;
    dbg!(*signal.get());

    let s = cx.create_signal(100);
    dbg!(*s.get());
    let t = spawn(move || {
        dbg!(*s.get());
    });
    dbg!(*s.get());
    *s.get_mut() = 300;
    t.join().unwrap();
}

[jumpnbrownweasel]

@romamik

The idea is that I can have read lock for the map and write lock for the signal.

Sorry, I overlooked that possibility.

I think your original implementation works quite well using two calls per access:

    dbg!(*signal.get().get());
    *signal.get().get_mut() = 200;
    dbg!(*signal.get().get());

This makes good use of temporaries to acquire two locks, and doesn't require using a (potentially risky) self-referential crate.

But of course it doesn't support a single struct with both guard locks, that can be moved around as a unit after locking. If that is a hard requirement, then I don't know of a solution other than using a self-referential crate, or wrapping each signal in an Arc.

For completeness, there is another advantage to wrapping each signal in an Arc: You can release the HashMap lock immediately after cloning the signal's Arc. This reduces the time the read lock is held on the HashMap, reducing blocking in threads that may be waiting to write lock the HashMap.