Mutually exclusive functions calling each other

Question

I have two functions foo and bar that should be mutually exclusive since they operate on the same data. However foo duplicates a lot of code from bar, so I would like to refactor foo to make a call to bar.

This is a problem because then I can't use a single mutex for both functions, because then foo would deadlock when it calls bar. So rather than "mutually exclusive" I only want "mutually exclusive from different threads".

Is there a pattern for implementing this? I'm using C++ and I'm okay with C++14/boost if I need something like shared_mutex.

Answer 1

Define a private "unlocked" function and use that from both foo and bar:

void bar_unlocked()
{
    // assert that mx_ is locked
    // real work
}

void bar()
{
    std::lock_guard<std::mutex> lock(mx_);
    bar_unlocked();
}

void foo()
{
    std::lock_guard<std::mutex> lock(mx_);
    // stuff
    bar_unlocked();
    // more stuff
}

Answer 2

another way - this has the advantage that you can prove that the lock has been taken:

void bar_impl(std::unique_lock<std::mutex> lock)
{
   assert(lock.owns_lock());
    // real work
}

void bar()
{
    bar_impl(std::unique_lock<std::mutex>(mx_));
}

void foo()
{
    // stuff
    bar_impl(std::unique_lock<std::mutex>(mx_));
    // more stuff
}

Rationale:

std::mutex is not (mandated by the standard to be) moveable, but a std::unique_lock<std::mutex> is. For this reason, we can move a lock into a callee and return it back to a caller (if necessary).

This allows us to prove ownership of the lock at every stage of a call chain.

In addition, once the optimiser gets involved, it's likely that all the lock-moving will be optimised away. This gives us the best of both worlds - provable ownership and maximal performance.

A more complete example:

#include <mutex>
#include <cassert>
#include <functional>

struct actor
{
  //
  // public interface
  //

  // perform a simple synchronous action
  void simple_action()
  {
    impl_simple_action(take_lock());
  }

  /// perform an action either now or asynchronously in the future
  /// hander() is called when the action is complete
  /// handler is a latch - i.e. it will be called exactly once
  /// @pre an existing handler must not be pending
  void complex_action(std::function<void()> handler)
  {
    impl_complex_action(take_lock(), std::move(handler));
  }

  private:

  //
  // private external interface (for callbacks)
  //
  void my_callback()
  {
    auto lock = take_lock();
    assert(!_condition_met);
    _condition_met = true;
    impl_condition_met(std::move(lock));
  }


  // private interface

  using mutex_type = std::mutex;
  using lock_type = std::unique_lock<mutex_type>;

  void impl_simple_action(const lock_type& lock)
  {
    // assert preconditions
    assert(lock.owns_lock());
    // actions here
  }

  void impl_complex_action(lock_type my_lock, std::function<void()> handler)
  {
    _handler = std::move(handler);
    if (_condition_met)
    {
      return impl_condition_met(std::move(my_lock));
    }
    else {
      // initiate some action that will result in my_callback() being called
      // some time later
    }
  }

  void impl_condition_met(lock_type lock)
  {
      assert(lock.owns_lock());
      assert(_condition_met);
      if(_handler)
      {
        _condition_met = false;
        auto copy = std::move(_handler);
        // unlock here because the callback may call back into our public interface
        lock.unlock();
        copy();
      }
  }



  auto take_lock() const -> lock_type
  {
    return lock_type(_mutex);
  }


  mutable mutex_type _mutex;

  std::function<void()> _handler = {};
  bool _condition_met = false;
};

void act(actor& a)
{
  a.complex_action([&a]{ 
    // other stuff...
    // note: calling another public interface function of a
    // during a handler initiated by a
    // the unlock() in impl_condition_met() makes this safe.
    a.simple_action(); 
  });

}