Thread safety/Transaction enforcer

I have some legacy classes written without thread safety in mind. Instances of these classes are now being accessed in a multithreaded context in a thread-un-safe manner. Cue chaos.

To fix this I make a wrapper class that enforces the user to take a transaction lock to serialize access to the instance.

```
#include
#include
#include
#include

namespace trapi {
template
class Transaction {
public:
Transaction(T& a_data, std::mutex& a_mutex)
: m_data(a_data), m_lg(a_mutex)
{}

Transaction(T& a_data, std::mutex& a_mutex, std::adopt_lock_t)
: m_data(a_data), m_lg(a_mutex, std::adopt_lock)
{}

Transaction(const Transaction&) = delete;

Transaction(Transaction&& a_that)
: m_data(a_that.m_data), m_lg(std::move(a_that.m_lg))
{}

~Transaction() = default;

Transaction& operator = (const Transaction& a_that) = delete;
Transaction& operator = (Transaction&& a_that) = delete;

T* operator -> () const {
assert(m_owner == std::this_thread::get_id());
return &m_data.get();
}

private:
std::reference_wrapper m_data;
std::unique_lock m_lg;

#ifndef NDEBUG
std::thread::id m_owner = std::this_thread::get_id();
#endif
};

// Starts multiple transactions simultaneously in a
// dead-lock free manner.
template
auto transaction(Args&&... args) {
std::lock(args...);
return std::make_tuple(args.adopt_transaction()...);
}

template
class TransactionEnforcer {
public:
template
TransactionEnforcer(Args&&... args)
: m_data(std::forward(args)...)
{}

TransactionEnforcer(const TransactionEnforcer&) = delete;
TransactionEnforcer(TransactionEnforcer&&) = delete;
~TransactionEnforcer() = default;
TransactionEnforcer& operator = (const TransactionEnforcer&) = delete;
Transact

Also the free function transaction that allows one to start multiple transactions without ending up in a deadlock. To be able to implement it was forced to make lock(), try_lock(), unlock() and adopt_transaction() public which I'm not happy about as I don't want the user fiddling with these and possibly breaking something by forgetting to unlock and what have you. I tried to declare the transaction function as a template friend but I couldn't figure out the return type. So I would like some help with that.

// Starts multiple transactions simultaneously in a 
// dead-lock free manner.
template
auto transaction(Args&&... args) {
    std::lock(args...);
    return std::make_tuple(args.adopt_transaction()...);
}

First of all, I don't think this is a good use of variadic templates. IIUC, you're trying to make a function that accepts any number of TransactionEnforcer objects and returns a std::tuple...>? I would express that at least as

template
auto transaction(TransactionEnforcer&&... args) {
    std::lock(args...);
    return std::make_tuple(args.adopt_transaction()...);
}

and possibly throw in the -> std::tuple...> since it's pretty easy to express; but bearing in mind that when you provide an explicit return type you enable SFINAE on that return type, which could theoretically turn some wrong code from a compiler error ("call to transaction is ambiguous...") into code that happily does the wrong thing (since one of the candidates is now SFINAE'd away). That's highly theoretical, though.

Now, how do you make this function a friend of TransactionEnforcer? That's easy; just define it inline!

template
class TransactionEnforcer
{
    // ...

    template
    friend auto transaction(TransactionEnforcer&&... args) {
        std::lock(args...);
        return std::make_tuple(args.adopt_transaction()...);
    }
};

This would allow you to make adopt_transaction a private member function; but it would not allow you to make lock, try_lock, and unlock private member functions, because they're still being touched by someone who's not a friend of the class; namely, they're being touched by std::lock().

To fix this, you could expose a private inner class LockView:

template
class TransactionEnforcer
{
    // ...
  private:
    void lock();
    bool try_lock();
    void unlock();
    friend struct LockView {
        TransactionEnforcer& te_;
        void lock() { return te_.lock(); }
        bool try_lock() { return te_.try_lock(); }
        void unlock() { return te_.unlock(); }
    };
    LockView lock_view() { return LockView{*this}; }

    template
    friend auto transaction(TransactionEnforcer&&... args) {
        std::lock(args.lock_view()...);
        return std::make_tuple(args.adopt_transaction()...);
    }
};

Unfortunately, this doesn't work out of the box because std::lock() requires that its arguments all be lvalues. So you'll have to make up some storage for the LockView objects somewhere: either use parameter-pack expansion to turn your parameter-pack full of TransactionEnforcers into a tuple full of LockViews suitable for passing (via std::apply?) to std::lock, or else make te.lock_view() return an lvalue reference to te.lv_, which is a self-referential LockView subobject of te. Fun times. But you can make it work.

No comment on the basic functionality of TransactionEnforcer; I'm not sure it's doing what you really want done, but maybe it is.

StackExchange Code Review Q#96195, answer score: 2