Returning wrapped iterator from RwLocked container

I have a container type protected by a RwLock. I'd like to allow functions to get read-only iterators to the container, which means my iter() function needs to acquire a read guard and pass it back to the caller, wrapping the iterator.

I thought this is just what lock_api::RwLockReadGuard::map() was for, but I cannot figure out the lifetimes I need here, or if I'm just way off.

use parking_lot::{RwLock,RwLockReadGuard,MappedRwLockReadGuard}; // 0.12.1
use std::collections::HashMap;

struct Container {
    item : RwLock::<HashMap::<String, usize>>,
}

impl Container {
    fn iter<'a : 'b, 'b>(&'a self) -> MappedRwLockReadGuard<'b, impl Iterator<Item = &'b usize>> {
        RwLockReadGuard::map(self.item.read(), |x| &x.values())
    }
}

(Playground)

Errors:

   Compiling playground v0.0.1 (/playground)
error: lifetime may not live long enough
  --> src/lib.rs:10:52
   |
10 |         RwLockReadGuard::map(self.item.read(), |x| &x.values())
   |                                                 -- ^^^^^^^^^^^ returning this value requires that `'1` must outlive `'2`
   |                                                 ||
   |                                                 |return type of closure is &std::collections::hash_map::Values<'2, String, usize>
   |                                                 has type `&'1 HashMap<String, usize>`

error[E0515]: cannot return reference to temporary value
  --> src/lib.rs:10:52
   |
10 |         RwLockReadGuard::map(self.item.read(), |x| &x.values())
   |                                                    ^----------
   |                                                    ||
   |                                                    |temporary value created here
   |                                                    returns a reference to data owned by the current function

For more information about this error, try `rustc --explain E0515`.
error: could not compile `playground` due to 2 previous errors

You can't solve this with naming your lifetimes, as this isn't really a lifetime problem but a problem with ownership and when values are dropped. Your callback |x| &x.values() returns a reference to an object that is created inside of the callback, which is impossible in Rust. x.values() creates an instance of Values, the type that implements your iterator over the values inside the hash map. This is dropped immediately after your callback finishes, so you can't return a reference to it (as it already doesn't exist no more). I don't see how you can solve this, as the map method of RwLockReadGuard needs to return a reference, which I don't understand why.

Edit: you can provide access to the hash map and make the caller call .values():

use parking_lot::{RwLock,RwLockReadGuard,MappedRwLockReadGuard}; // 0.12.1
use std::collections::HashMap;

struct Container {
    item : RwLock::<HashMap::<String, usize>>,
}

impl Container {
    fn new(map: HashMap<String, usize>) -> Self {
        Self { item: RwLock::new(map) }
    }
    
    fn inner(&self) -> MappedRwLockReadGuard<'_, HashMap<String, usize>> {
        RwLockReadGuard::map(self.item.read(), |x| x)
    }
}

fn main() {
    let map = HashMap::from([
        ("Foo".to_owned(), 1),
        ("Bar".to_owned(), 2),
    ]);
    
    let c = Container::new(map);
    
    for v in c.inner().values() {
        println!("{}", v);
    }
}

Playground.

Because GAT-like Fn traits don't exist and the stable traits require mentioning the output type, basically.

    fn inner(&self) -> RwLockReadGuard<'_, HashMap<String, usize>> {
        self.item.read()
    }

I think it could just as well return a RwLockReadGuard itself.

This is maybe a more meaningful step in the right direction:

Basically, I'd like to expose a bunch of thread-safe operations inside the wrapper that all operate safely while holding the appropriate locks, and I don't want to do it with just monolithic functions but with a series of private functions that return MappedRwLockReadGuards of data in the container.

Why return a reference to a generic type though? This greatly reduces the possibilities of the callback in my opinion, like in this example. I haven't looked at the implementation, so I'm probably missing something. But on first glance it looks weird to me.

Funnily enough, I was given a type like your Container in an interview question once and I should implement some sort of iteration over the locked data as well (though my item was a RwLock<Vec<_>>, not a hash map and I didn't use parking_lot :slightly_smiling_face:). The solution they expected me to find was to basically implement a for_each method that takes a callback, hiding the fact that the underlying data is synchronized. So basically:

use parking_lot::RwLock;
use std::collections::HashMap;

struct Container {
    item : RwLock::<HashMap::<String, usize>>,
}

impl Container {
    fn new(map: HashMap<String, usize>) -> Self {
        Self { item: RwLock::new(map) }
    }
    
    fn for_each<F>(&self, f: F) where F: Fn(&usize) {
        let lock = self.item.read();
        
        for v in lock.values() {
            f(v);
        }
    }
}

fn main() {
    let map = HashMap::from([
        ("Foo".to_owned(), 1),
        ("Bar".to_owned(), 2),
    ]);
    
    let c = Container::new(map);
    
    c.for_each(|v| println!("{}", v));
}

Playground.

I must say that I find this kind of interface quite readable and easy to understand, especially compared to passing guards to the caller.

Because you want to capture the input lifetime, but Rust has no generic type constructors, either. Returning a reference at least captures the common case of field projection.

Potentially derailing exploration

Given a locked T, we're going to be dealing with &T (and interior mutability if needed), since we're talking about a concurrent data structure. And the staple use-case of projection is to go from a struct to a field of that struct. So that means we're going to be going from &T -> &U.

However, in this case, we want to go not a field, but to something constructed from a field (or &T). We want &T -> Values<'_, K, V>. This time, that is -- maybe some other time we'd want &T -> Keys<'_, K, V>.

How do you write a trait bound for a closure that takes a &T and hands back a lifetime-capturing, but otherwise generic, type?

impl<'a, R: RawRwLock + 'a, T: ?Sized + 'a> RwLockReadGuard<'a, R, T> {
    pub fn map<U: ?Sized, F>(s: Self, f: F) -> MappedRwLockReadGuard<'a, R, U>
    where
        F: FnOnce(&T) -> ????????,
// Goal: Support &U, Keys<'_, K, V>, Values<'_, K, V>, etc

There's currently no direct way to do so on stable.

  • You can't use a type parameter, because that must resolve to a single type
    • And types that vary by lifetime are not types, they are type constructors (including &U)
    • So a FnOnce(&T) -> &U is not a FnOnce(&T) -> V
     // Won't work, `U` can't capture the input lifetime
     pub fn map<U: ?Sized, F>(s: Self, f: F) 
         -> MappedRwLockReadGuard<'a, R, *U>
     where
         F: FnOnce(&T) -> U,
    
    • And stable Rust doesn't let you omit the return type in a FnOnce bound
     // Won't work due to using the `FnOnce<(&'_ T,)>` syntax
     // And there's probably difficulty in pulling out the correct return
     // type in this particular case (it stores a `*const U` currently)
     pub fn map<F>(s: Self, f: F) 
         -> MappedRwLockReadGuard<'a, R, <F as FnOnce<(&'??? T,)>>::Output>
     where
         for<'any> F: FnOnce<(&'any T,)>,
    
  • Rust has no generic type constructors
    // Won't work, no such thing as a `U<*>`
    pub fn map<U<'_>, F>(s: Self, f: F) 
        -> MappedRwLockReadGuard<'a, R, U<'*>>
      where
          F: FnOnce(&T) -> U<'_>,
    
    // Won't work, not supported and no way to name the
    // opaque return type to boot
    pub fn map<F>(s: Self, f: F) 
        -> MappedRwLockReadGuard<'a, R, ???>
      where
          F: FnOnce(&T) -> impl Sized + '_,
    
  • There's no generic way to erase lifetimes, like there is for references
    • Relevant to this particular example since it stores *const U

You can use helper traits to emulate FnOnce<(&'_ T,)> on stable, but it tends to wreck inference. And probably also doesn't address the lifetime-erasing aspect of this particular example.

1 Like

That completely makes sense, thank you!