0

Suppose you have a data structure where values are kept in nodes, e.g., the standard maps or lists, and that structure has a lock to guard it. Suppose also the value types (mapped_type for the maps) is complex enough that each one has its own lock.

So the basic operation is to grab the map's lock, get the (reference to the) value, and unlock the map's lock. Now you can lock the value's lock and party on it as long as you want.

The problem - as I see it - is you can't assign a reference - only initialize it. So I propose the following sort of code, given the some map type M with instance m (and it's mutex m_mutex):

const MappedType& mt = [this, key]() -> const MappedType& {
  std::lock_guard _{m_mutex};
  return m[key];
}();

That would be the case where you can use at to add an element. If you didn't want to do that you could:

const MappedType& mt = [this, key]() -> const MappedType& {
  std::lock_guard _{m_mutex};
  M::const_iterator it = m.find(key);
  if (m.cend() != it)
    return it->second;
  else
    return M::default_m;   // some static M to indicate 'missing', or maybe you throw
}();

Good idea or too clever to pass a code review? What would the idiomatic way to do this in modern C++ be?

davidbak
  • 5,775
  • 3
  • 34
  • 50
  • I've learned a lot from the harsh guidance Rust gives and I think this would fail the borrow checker really hard. Return a copy to be safe. Return a reference if you like living dangerously. – tadman Mar 18 '21 at 06:23
  • @tadman - can't return a copy: the things being held in the map each have their own mutex ... they're not copyable. Lifetime is not an issue, so references are not a problem. – davidbak Mar 18 '21 at 06:25
  • Well, *if you say so*. I'm just saying from a code review perspective that's an assertion that needs more guarantees than your word. Rust is extraordinarily strict about this kind of stuff. Learning from that, I'd recommend taking in a `lambda` to do any work on this thing to guarantee it doesn't out-live the lifespan of the object being referenced. Anything that invalidates pointers or iterators here could ruin your parade. You hold that lock *until the work is done* and there's not going to be any funny business. – tadman Mar 18 '21 at 06:25
  • 1
    Are we removing from the map as well? Does removal lock the element? Any reason not to use shared_ptr? – StoryTeller - Unslander Monica Mar 18 '21 at 06:26
  • What's actually the question? The first code indeed makes sense when mapped type is a lockable. The second one is not good. Don't ever return reference to some weird object instance. If return can fail you should return pointer or iterator - definitely not a reference; you can throw an exception too. Those are just basic guidelines. – ALX23z Mar 18 '21 at 06:31
  • What's wrong with pointers? Returning a pointer could pass code review, assuming that the data held within the map is indeed const, and had no chance of being erased by some other thread. Otherwise, you have just created a bug. – Michaël Roy Mar 19 '21 at 01:06

0 Answers0