6

Current situation

Right now I have a method like

  Data lookupData(Key id)
  {
    std::lock_guard<std::mutex> lock(m_mutex);
    auto it = m_dict.find(id);
    if(it == m_dict.end())
    {
      return Data::Empty;
    }
    else
    {
      return it->second;
    }
  }

Problem

However I now have a situation where, if the result is not found, I want the calling code to be able to do some more stuff while keeping the lock held.

One straightforward solution is to take the mutex lock outside of this method and require calling code to lock the mutex instead, maybe by documenting this in a doc comment or something. However it feels like it would be better to be able to enforce this, by passing some kind of receipt to the method proving that we actually hold a lock on the mutex in question.

Potential solution(s)

So I'm considering doing something like

  Data lookupData(Key id, const std::unique_lock<std::mutex> & lock)
  {
    if(lock.mutex() != m_mutex) { /* throw * }
    if(! lock.owns_lock()) { /* throw */ }

    auto it = m_dict.find(id);
    if(it == m_dict.end())
    {
      return Data::Empty;
    }
    else
    {
      return it->second;
    }
  }

Alternatively instead of

     if(! lock.owns_lock()) { /* throw */ }

I could do something like

     if(! lock.owns_lock()) { lock.lock(); }

(obviously removing the const qualifier from the lock parameter) so that the method can be called either with or without the lock being held.

Questions

  • Is the idea of a method requiring proof that a mutex is locked a good one, or if not, why not?
  • Assuming the answer to the above is "yes," are the above approaches recommended? If not, why not, and what would be recommended instead?
  • Are there names for the type of problem and/or the general solutions described above that I can Google to read more about this sort of thing?
Daniel McLaury
  • 352
  • 1
  • 7
  • 2
    Could you make your lock reentrant, and lock from the caller of the function and from the function as well? – Vincent Savard Nov 29 '21 at 15:07
  • 2
    Have you considered recursive mutexes? Those can be locked multiple times by one thread. – Bart van Ingen Schenau Nov 29 '21 at 15:07
  • I haven't heard of recursive_mutex before. It's an interesting idea but (without any experience with them) sounds like it could easily open up a whole can of worms by letting people handle mutexes fairly lazily. – Daniel McLaury Nov 29 '21 at 15:24
  • 1
    Concurrency is indeed hard, but the usage of reentrant locks is quite widespread when the use case warrants it. I think what could also be helpful to answer your question is to have a more practical example of why you need the caller to hold the lock, and how the `lookupData` method is intended to be used (Can it be called from other classes?). – Vincent Savard Nov 29 '21 at 16:23
  • 1
    You should always, always hold a mutex for the minimum time possible. Look for recursive mutexes, and lock if you have a try lock functionality in your programming environment. Checking whether a mutex is locked and then locking it is unsafe. – gnasher729 Nov 29 '21 at 21:06
  • 1
    Daniel, lazy is good for software developers. – gnasher729 Nov 30 '21 at 08:30

1 Answers1

2

One solution here is to create a proxy object for the API that should only be available while locked. Something like this:

class Dict
{
  std::mutex m_mutex;
  std::unordered_map<Key, Data> m_dict;
public:
  class Lock
  {
    std::unique_lock<std::mutex> m_lock;
    Dict* m_parent;
  public:
    explicit Lock(Dict& parent)
      : m_lock(parent.m_mutex),
        m_parent(&parent)
    {}
    Data lookup(Key key)
    {
      auto it = m_parent->m_dict.find(id);
      return it == m_parent->m_dict.end() ? Data::Empty : it->second;
    }
  };
  Lock lock()
  { return Lock(*this); }
};

int main()
{
  Dict d;
  d.lock().lookup(0);
  auto locked = d.lock();
  if(locked.lookup(1))
    locked.lookup(2);
}
Homer512
  • 121
  • 1