5

I would like to have a class member function which returns a pointer to a resource. The resource should be locked and unlocked automatically. I think of creating a non-copyable object which handles the locking.

Do you think the following is a good solution? Is it thread-safe? Are there already tools in the STL for this use case?

template<typename T, typename M>
struct LockedResource
{
private:
    T* data_;
    std::unique_lock<std::mutex> lock_;

public:
    LockedRessource(T* data, M& mtx) : data_{data}, lock_{mtx} {}
    T* data() const { return data_; }
};

Use case example:

#include <iostream>
#include <mutex>
#include <thread>
#include <vector>

class Foo {
private:
    std::vector<int> data_;
    std::mutex mtx_;
public:
    LockedResource<std::vector<int>,std::mutex> data()
    { return LockedResource<std::vector<int>,std::mutex>{&data_, mtx_}; }
};

Foo foo;

void worker(int worker, int iterations, int dt) {
    for(int i=0; i<iterations; i++) {
        std::this_thread::sleep_for(std::chrono::milliseconds(dt));
        auto res = foo.data();
        // we now have a unique_lock until the end of the scope
        std::cout << "Worker " << worker << " adding " << i << std::endl;
        res.data()->push_back(i);
    }
}

int main() {
    std::thread t1{worker, 1, 10, 173};
    std::thread t2{worker, 2, 20, 87};    
    t1.join();
    t2.join();
}
Danvil
  • 22,240
  • 19
  • 65
  • 88
  • 1
    Consider also a `std::unique_ptr` instance with a custom deleter. – Rook May 12 '14 at 13:38
  • @Rook Could you elaborate that idea a bit? – Danvil May 12 '14 at 13:55
  • 2
    Following a little though, I'm no longer certain its a great idea. The problem is, a custom deleter needs to be copyable, so it cannot own anything like `std::unique_lock`. This means the customer deleter must hold a reference to the `Foo` class that owns the data resource, and be able to manipulate its `mtx_` property directly. This doesn't give me a good feeling, and I'm concerned about the resulting code's thread and exception safety. I'll think about it a little more. – Rook May 12 '14 at 15:08
  • 1
    @Rook A customer deleter for `unique_ptr` need not be copyable, movability is sufficient. – Casey May 12 '14 at 15:12
  • @Casey so I thought, but I seem to be quite incapable of creating a deleter containing, say a `unique_lock` without getting errors due to deleted functions. Perhaps I should ask a separate question on that matter. – Rook May 12 '14 at 15:17
  • @Casey turns out I was suffering from a compiler/standard library issue with VS2013 (gcc was unaffected). I'd add an answer now, but it seems you've beaten me to it ;) – Rook May 12 '14 at 16:04

1 Answers1

4

This idea - of providing a handle that encapsulates both access to a synchronized object and the necessary locking - is not new: see Enforcing Correct Mutex Usage with Synchronized Values.

I liked Rook's idea to use a unique_ptr with a custom deleter for the handle, so I came up with one:

template <typename BasicLockable>
class unlock_deleter {
    std::unique_lock<BasicLockable> lock_;
public:
    unlock_deleter(BasicLockable& mtx) : lock_{mtx} {}
    unlock_deleter(BasicLockable& mtx, std::adopt_lock_t a) noexcept : lock_{mtx, a} {}

    template <typename T>
    void operator () (T*) const noexcept {
        // no-op
    }
};

template <typename T, typename M = std::mutex>
using locked_ptr = std::unique_ptr<T, unlock_deleter<M>>;

which is used in the implementation of a template class synchronized that wraps an object and a mutex:

template <typename T, typename M = std::mutex>
class synchronized {
  T item_;
  mutable M mtx_;

  // Implement Copy/Move construction
  template <typename Other, typename N>
  synchronized(Other&& other, const std::lock_guard<N>&) :
    item_{std::forward<Other>(other).item_} {}

  // Implement Copy/Move assignment
  template <typename Other>
  void assign(Other&& other) {
    std::lock(mtx_, other.mtx_);
    std::lock_guard<M> _{mtx_, std::adopt_lock};
    std::lock_guard<decltype(other.mtx_)> _o{other.mtx_, std::adopt_lock};
    item_ = std::forward<Other>(other).item_;
  }

public:
  synchronized() = default;

  synchronized(const synchronized& other) :
    synchronized(other, std::lock_guard<M>(other.mtx_)) {}
  template <typename N>
  synchronized(const synchronized<T, N>& other) :
    synchronized(other, std::lock_guard<N>(other.mtx_)) {}

  synchronized(synchronized&& other) :
    synchronized(std::move(other), std::lock_guard<M>(other.mtx_)) {}    
  template <typename N>
  synchronized(synchronized<T, N>&& other) :
    synchronized(std::move(other), std::lock_guard<N>(other.mtx_)) {}    

  synchronized& operator = (const synchronized& other) & {
    if (&other != this) {
      assign(other);
    }
    return *this;
  }
  template <typename N>
  synchronized& operator = (const synchronized<T, N>& other) & {
    assign(other);
    return *this;
  }

  synchronized& operator = (synchronized&& other) & {
    if (&other != this) {
      assign(std::move(other));
    }
    return *this;
  }
  template <typename N>
  synchronized& operator = (synchronized<T, N>&& other) & {
    assign(std::move(other));
    return *this;
  }

  template <typename N>
  void swap(synchronized<T, N>& other) {
    if (static_cast<void*>(&other) != static_cast<void*>(this)) {
      std::lock(mtx_, other.mtx_);
      std::lock_guard<M> _{mtx_, std::adopt_lock};
      std::lock_guard<N> _o{other.mtx_, std::adopt_lock};

      using std::swap;
      swap(item_, other.item_);
    }
  }

  locked_ptr<T, M> data() & {
    return locked_ptr<T, M>{&item_, mtx_};
  }
  locked_ptr<const T, M> data() const& {
    return locked_ptr<const T, M>{&item_, mtx_};
  }
};

template <typename T, typename M, typename N>
void swap(synchronized<T, M>& a, synchronized<T, N>& b) {
  a.swap(b);
}

Taking care to synchronize copies/moves/swaps properly as well. Here's your sample program live at Coliru.

Community
  • 1
  • 1
Casey
  • 41,449
  • 7
  • 95
  • 125
  • Somewhat more thorough implementation than the one I'd put together. Nice to know my ideas are sometimes worthwhile ;) – Rook May 12 '14 at 16:07