Is this RAII exclusive resource checkout object / manager combination thread safe?

Question

I want to write a wrapper that allows to exclusively checkout a resource and automatically returns the resource once the holder goes out of scope.

The usage in the (multi-threaded) client code is like this:

{
  auto myResource(_manager.checkoutExclusive());

  doStuffWithoutEverStoringTheValue(myResource.val());
} //resource is returned to _manager

I know that handing an IAlgorithm to _manager to be executed on resource would be cleaner, but for the moment that is not the optimal solution for me.

I also want to provide a parallel checkout option that allows several concurrent checkouts (e.g., for read only access, or if the client handles thread safety for the resource itself). Of course parallel and exclusive checkout can not be mixed.

Following is my attempt to solve this. Aside from the duplicated code that I could not avoid (const/non-const versions of checkout), or did not want to avoid for clarity (ExclusiveCheckout/ParallelCheckout), my main question is:

Is this thread safe? (I'm pretty sure I missed a subtle race condition.)

#include <mutex>
#include "global/ScopeGuard11.h" //Using the folly::ScopeGuard

//Forward declarations for friend statements.
template<typename T>
class ExclusiveCheckout;

template<typename T>
class ParallelCheckout;

template<typename T, typename TConst = T const>
class CheckoutValue;

// Interface for returning an exclusively checked out resource
class IReturnExclusive
{
  template<typename T>
  friend class ExclusiveCheckout;

  protected:

    virtual void returnExclusive() const = 0;

};

// Holder for the exclusively checked out resource. Only move construction and val() are public.
// Destruction returns the resource.
template<typename T>
class ExclusiveCheckout
{
  template<typename T, typename TConst>
  friend class CheckoutValue;

  public:

    ExclusiveCheckout(ExclusiveCheckout<T> &&src)
    : _returnTo(src._returnTo), _value(src._value)
    {
      src._returnTo = nullptr;
      src._value = nullptr;
    }

    ~ExclusiveCheckout()
    { 
      if (_returnTo)
      {
        _returnTo->returnExclusive();
      }
    }

    virtual T &val() final
    { 
      return *_value;
    }


  private:

    IReturnExclusive const *_returnTo;
    T *_value;

    ExclusiveCheckout(IReturnExclusive const &returnTo, T &value)
    : _returnTo(&returnTo), _value(&value)
    { }

    ExclusiveCheckout(ExclusiveCheckout<T> const &src); //disallowed
    ExclusiveCheckout const &operator=(ExclusiveCheckout<T> const &other); //disallowed
};


// Interface for returning a parallely checked out resource
class IReturnParallel
{
  template<typename T>
  friend class ParallelCheckout;

  protected:

    virtual void returnParallel() const = 0;

};

// Holder for the parallely checked out resource. Only move construction and val() are public.
// Destruction returns the resource.
template<typename T>
class ParallelCheckout
{
  template<typename T, typename TConst>
  friend class CheckoutValue;

  public:

    ParallelCheckout(ParallelCheckout<T> &&src)
    : _returnTo(src._returnTo), _value(src._value)
    {
      src._returnTo = nullptr;
      src._value = nullptr;
    }

    ~ParallelCheckout()
    { 
      if (_returnTo)
      {
        _returnTo->returnParallel();
      }
    }

    virtual T &val() final
    {
      return *_value;
    }


  private:

    IReturnParallel const *_returnTo;
    T *_value;

    ParallelCheckout(IReturnParallel const &returnTo, T &value)
    : _returnTo(&returnTo), _value(&value)
    { }

    ParallelCheckout(ParallelCheckout<T> const &src); //disallowed
    ParallelCheckout const &operator=(ParallelCheckout<T> const &other); //disallowed
};


// The resource manager.
template<typename T, typename TConst>
class CheckoutValue final : protected IReturnExclusive,
                            protected IReturnParallel
{

public:

  CheckoutValue()
  : _checkoutValue(), _parallelCnt(0)
  { }

  CheckoutValue(T const &checkoutValue)
  : _checkoutValue(checkoutValue), _parallelCnt(0)
  { }

  virtual ~CheckoutValue() { };

  void setValue(T const &checkoutValue)
  {
    // Only change the resource if noone is using it
    std::lock_guard<std::mutex> guard(_exclusiveMutex);
    _checkoutValue = checkoutValue;
  }


  ExclusiveCheckout<T> checkoutExclusive()
  {
    ScopeGuard guard = folly::makeGuard([&] { _exclusiveMutex.unlock(); });
    _exclusiveMutex.lock();

    ExclusiveCheckout<T> ret(*this, _checkoutValue);

    guard.dismiss();

    return ret;
  }


  ExclusiveCheckout<TConst> checkoutExclusive() const
  {
    ScopeGuard guard = folly::makeGuard([&] { _exclusiveMutex.unlock(); });
    _exclusiveMutex.lock();

    ExclusiveCheckout<TConst> ret(*this, _checkoutValue);

    guard.dismiss();

    return ret;
  }


  ParallelCheckout<T> checkoutParallel()
  {
    std::lock_guard<std::mutex> guardParallel(_parallelMutex);

    ScopeGuard guard = folly::makeGuard([&] { _exclusiveMutex.unlock(); });
    if (_parallelCnt == 0)
    {
      _exclusiveMutex.lock();
    }
    else
    {
      guard.dismiss();
    }

    ParallelCheckout<T> ret(*this, _checkoutValue);

    ++_parallelCnt;

    guard.dismiss();

    return ret;
  }


  ParallelCheckout<TConst> checkoutParallel() const
  {
    std::lock_guard<std::mutex> guardParallel(_parallelMutex);

    ScopeGuard guard = folly::makeGuard([&] { _exclusiveMutex.unlock(); });
    if (_parallelCnt == 0)
    {
      _exclusiveMutex.lock();
    }
    else
    {
      guard.dismiss();
    }

    ParallelCheckout<TConst> ret(*this, _checkoutValue);

    ++_parallelCnt;

    guard.dismiss();

    return ret;
  }


protected:

  virtual void returnExclusive() const final override
  {
    _exclusiveMutex.unlock();
  }

  virtual void returnParallel() const final override
  {  
    std::lock_guard<std::mutex> guardParallel(_parallelMutex);

    --_parallelCnt;

    if (_parallelCnt == 0)
    {
      _exclusiveMutex.unlock();
    }
  }


private:

  mutable std::mutex _exclusiveMutex;
  mutable std::mutex _parallelMutex;
  mutable int _parallelCnt;

  T _checkoutValue;

};

The entire flow control `_returnTo` is not thread safe. – Aug 20 '15 at 17:28 — , Aug 20 '15 at 17:28
But ExclusiveCheckout only lives in one thread, doesn't it? – B3ret Aug 20 '15 at 17:33 — B3ret, Aug 20 '15 at 17:33

score 2 · Accepted Answer · edited May 23 '17 at 12:14

In C++14 this is very short. And easy to confirm it is correct.

Implementing the pieces of C++14 you need, instead of writing it manually, is probably best.

// holds a "reference" to a T and a Lock
// can be moved-from, in which case it holds nothing:
template<class T, class Lock>
struct locked_resource:Lock {
  locked_resource( T& t, Lock&& lock ):
    Lock(std::move(lock)),
    data(std::addressof(t))
  {}

  locked_resource(locked_resource&&o):Lock(std::move(o)),data(o.data){
    o.data=nullptr;
  }

  // use pointer-semantics for "is valid" and "get" via operator*:
  T& operator*() const { return *data; }
  explicit operator bool()const{ return data; }
private:
  T* data;
};

// shared and exclusive lock types
// also used as shared and exclusive checkout tags:
using shared = std::shared_lock<std::shared_timed_mutex>;
using exclusive = std::unique_lock<std::shared_timed_mutex>;
template<class T>
struct protected_resource {
  // access methods:
  template<class Access>
  locked_resource<T, Access> checkout(){
    return access<T,Access>(*this);
  }
  template<class Access>
  locked_resource<T const, Access> checkout() const {
    return access<T const,Access>(*this);
  }

  // ctors:
  protected_resource()=default;
  template<class U>
  explicit protected_resource(U&& u):t(std::forward<U>(u)) {}

  // setter.  I perfect forward:
  template<class U>
  void set(U&& u) {
    auto x = checkout<exclusive>();
    *x = std::forward<U>(u);
  }
  // for .set({blah}) support:
  void set(T&& t) {
    set<T>(std::move(t));
  }
private:
  // hey look, a one-liner!  This, plus 2 other one-liners
  // replaces 37 lines of 4 functions source in OP: (not counting whitespace)
  template<class U, class Lock, class Self>
  friend locked_resource<U, Lock> access(Self& self) {
    return {t, Lock(m)};
  }
  std::shared_timed_mutex m; // or shared_mutex
  T t;
};

which removes all the messy shared/exclusive code from the resource management.

Now simply implement shared_timed_mutex and shared_lock. Because they are no longer entangled with your business logic, you are going to have an easier time.

You could steal an implementation from Howard, for example, or from boost.

Example use:

protected_resource<int> _manager;
{
  auto myResource = _manager.checkout<exclusive>();

  doStuffWithoutEverStoringTheValue(*myResource);
} //resource is returned to _manager

Is this RAII exclusive resource checkout object / manager combination thread safe?

1 Answers1