Trying to avoid code duplication for some specific C++ code

Question

I have some C++ code which consists of repeats of the following pattern:

int Func(Type1 arg1, Type2 arg2)
{
  RAIILock lock(Singleton::Mutex());
  Instance* ptr = GetClassInstance();
  if (ptr) {
    return ptr->Func(arg1, arg2);
  } else {
    return -1;
  }
}

Basically, it's trying to get a valid class instance pointer under the lock, and essentially forwarding the call from this normal function to an instance method with the same signature. The "Func" name as well as amount and type of arguments vary, but the rest of the calls are identical.

It feels like there should be some way to achieve this using templates without getting into too much template magic, but I've been unable to come up with anything.

Answer 1

I would start with this primitive:

template<class F>
std::result_of_t<F(Instance*)> while_locked( F&& f ) {
  RAIILock lock(Singleton::Mutex());
  Instance* ptr = GetClassInstance();
  if (ptr)
    return std::forward<F>(f)(ptr):
  else
    return -1;
}

use:

int x = while_locked( [&](auto* ptr) {
  ptr->foo( a, b );
} );

basically, you get the ability to do whatever you want while locked, and have access to a ptr while doing it. This lets you merge multiple method calls into one atomic operation.

This can be extended to a multi-ary while-locked, that does an ordering of mutexes on a set of ptr-sources, locks them appropriately, gets the pointers from each, then invokes your function.

It also becomes easy to write your "invoke a member function" version, but restricting your atomic operations to "only call some specific member function" seems questionable.

I will admit the syntax is nicer in C++14 than in C++11 with auto-lambda. But most major compilers have implemented auto-lambda by this point.

There is a more general pattern that looks like this:

template<class T>
struct lockable {
  template<class F>
  std::result_of_t<F(T const&)> read( F&& f ) const {
    auto l = lock();
    return std::forward<F>(f)(t);
  }
  template<class F>
  std::result_of_t<F(T&)> write( F&& f ) {
    auto l = lock();
    return std::forward<F>(f)(t);
  }
private:
  using read_lock_t = std::shared_lock<std::shared_mutex>;
  using write_lock_t = std::unique_lock<std::shared_mutex>;

  read_lock_t lock() const { return read_lock_t(m); }
  write_lock_t lock() { return write_lock_t(m); }

  mutable std::shared_mutex m;
  T t;
};

which can be augmented with operator=, copy-ctor, move-ctors, default-ctors, and multi-locking capabilities. Multi-locking comes from std::lock and creation of unlocked lockables.

  read_lock_t unlocked() const { return {m, std::defer_lock}; }
  write_lock_t unlocked() { return {m, std::defer_lock}; }

  template<size_t...Is, class...Selfs>
  friend auto lock_all( std::index_sequence<Is...>, Selfs&... selfs ) {
    auto r = std::make_tuple( selfs.unlocked() );
    std::lock( std::get<Is>(r)... );
    return r;
  }
  template<class...Selfs>
  friend auto lock_all( Selfs&... selfs ) {
    return lock_all( std::index_sequence_for<Selfs...>{}, selfs... );
  }

public:
  template<class F, class...Selfs>
  friend auto invoke( F&& f, Selfs&...selfs )
  -> std::result_of_t< F( decltype(selfs.t)... ) >
  {
    auto l = lock_all(selfs...);
    return std::forward<F>(f)(selfs.t...);
  }

which lets you do invoke( lambda, a, b, c ) which will lock a b and c using the appropriate lock for their const-ness, and then invoke the lambda.

This allows easy writing of operator= for example:

lockable& operator=( lockable&& o ) {
  if (this = std::addressof(o)) return *this;
  return invoke( [](T& target, T& src){
    return target = std::move(src);
  }, *this, o );
}
lockable& operator=( lockable const& o ) {
  if (this = std::addressof(o)) return *this;
  return invoke( [](T& target, T const& src){
    return target = src;
  }, *this, o );
}

Answer 2

You might not apply too much convenience and have a guarded singleton allowing multiple member function calls:

#include <mutex>
template <typename T>
class Lockable
{
    template <typename> friend class Lock;

    public:
    typedef std::mutex mutex_type;
    typedef std::lock_guard<std::mutex> lock_guard;

    public:
    Lockable()
    {}

    template <typename A>
    Lockable(A&& a)
    : _value(std::forward<A>(a))
    {}

    template <typename A, typename ... Args>
    Lockable(A&& a, Args&& ... args)
    : _value(std::forward<A>(a), std::forward<Args>(args)...)
    {}

    Lockable(const Lockable&) = delete;
    Lockable& operator = (const Lockable&) = delete;

    explicit operator T () const {
        lock_guard lock(_mutex);
        T result = _value;
        return result;
    }

    Lockable& operator = (const T& value) {
        lock_guard lock(_mutex);
        _value = value;
        return *this;
    }

    private:
    mutable mutex_type _mutex;
    T _value;
};

template <typename T>
class Lock
{
    public:
    typedef Lockable<T> lockable_type;
    typedef typename lockable_type::mutex_type mutex_type;
    typedef typename lockable_type::lock_guard lock_guard;

    public:
    Lock(lockable_type& lockable)
    : _lock(lockable._mutex), _ptr(&(lockable._value))
    {}

    Lock(const Lock&) = delete;
    Lock& operator = (const Lock&) = delete;

    operator T& () const { return *_ptr; }
    T& operator *  () const { return *_ptr; }
    T* operator -> () const { return _ptr; }

    private:
    lock_guard _lock;
    T* _ptr;
};

class Singleton
{
    private:
    friend class Lockable<Singleton>;

    Singleton() {};

    public:
    Singleton(const Singleton&) = delete;
    Singleton& operator = (const Singleton&) = delete;

    static Lockable<Singleton>& instance();

    int Func0(int arg) const { return 0; }
    int Func1(int arg) const { return 1; }
    int Func2(int arg) const { return 2; }
};

Lockable<Singleton>& Singleton::instance() {
    static Lockable<Singleton> result;
    return result;
}

#include <iostream>

int main() {
    Lock<Singleton> singleton(Singleton::instance());
    singleton->Func0(0);
    singleton->Func1(1);
    singleton->Func2(2);
    std::cout << "Reached\n";
    // Uncomment to get a deadlock
    // Lock<Singleton> deadlock_singleton(Singleton::instance());
    // std::cout << "Not reached\n";
    return 0;
}

Note: The Lock<Singleton> singleton(Singleton::instance()); is clumsy, due to the non moveable std::lock_guard.

Answer 3

Something like that?

template <class MEM_FUN, class... ARGS>
auto call_func(MEM_FUN&& f, ARGS&&... args)
{
  RAIILock lock(Singleton::Mutex());
  Instance* ptr = GetClassInstance();
  if (ptr) {
    return (ptr->*f)(std::forward<ARGS>(args)...);
  } else {
    return -1;
  }
}

Calling like this:

call_func(&Instance::Func, arg1, arg2, arg3);