C++ templates - Generalized operations and visibility of members between instantations

Question

As some practice, I decided to implement a non-complete threadsafe and generic stack. You can see an initial declaration of the class template below:

template <typename T>
class ThreadsafeStack {
public:
  ThreadsafeStack() = default;

  ThreadsafeStack(ThreadsafeStack<T> const&);
  template <typename U>
  ThreadsafeStack(ThreadsafeStack<U> const&);

  ~ThreadsafeStack() = default;

  ThreadsafeStack<T>& operator=(ThreadsafeStack<T> const&);
  template <typename U>
  ThreadsafeStack<T>& operator=(ThreadsafeStack<U> const&);

  void push(T new_value);
  std::shared_ptr<T> pop();
  void pop(T& value);
  bool empty() const;
private:
  std::stack<T> data;
  mutable std::mutex m;
};

My problem is about the generalized copy operations (for simplicity, I left out the move operations for now). As an example, see the definition of the "normal" and generalized assignment operators:

// "Normal"
template <typename T>
ThreadsafeStack<T>& ThreadsafeStack<T>::operator=(ThreadsafeStack<T> const& o)
{
  if ((void*) this == (void*) &o) {
    return *this;
  }

  std::lock(m, o.m);
  std::lock_guard<std::mutex> lock1(m, std::adopt_lock);
  std::lock_guard<std::mutex> lock2(o.m, std::adopt_lock);

  data = o.data;

  return *this;
}

// "Generalized"
template <typename T>
template <typename U>
ThreadsafeStack<T>& ThreadsafeStack<T>::operator=(ThreadsafeStack<U> const& o)
{
  if ((void*) this == (void*) &o) {
    return *this;
  }

  // ... ?

  return *this;
}

With this code, the problem is that the member std::mutex m of ThreadsafeStack<U> is not visible from ThreadsafeStack<T> - so I can lock m but not o.m (unlike in the "normal" case).

(Note that I have to lock the mutex of the data structure to be copied without breaks during the whole copying process. I don't think that locking and releasing by elements would be either efficient or logically correct. Consider that I have a stack consisting of values 5, 6, 7; first, I copy 5 but after that another thread pops 6 so the next element I copy is 7...)

So, how would you solve the problem above?

I already considered the following solutions:

make the mutex member public (worst idea, don't want to do that). (Returning a reference to them with a member function is a similar bad OO design idea in my opinion.)
Second one: create a public member function that returns a copy of the complete underlying data (note that this way locking must be performed only once). I think a longer, but similar workaround could be to use the already present interface.
Doesn't exist there any language provided, more natural way to do this? (For example, some special syntax that denotes that all instantiations of a specific template see each other's private and protected members for such purposes.)

Also tried to do some research in the topic:

in C++ Templates - The Complete Guide the authors also pointed out in Chapter 5 that because of this problem they had to use the public interface to make the copy. Despite I find this a great book, unfortunately it's a little bit old so there might be more efficient solutions for this nowadays.
This might be also true (in connection with certain items) for Scott Meyer's Effective C++: Item 45 mentioned "normal" and generalized copy constructors/assignment operators, but it wasn't deep enough in this specific direction.
Read similar questions on SO but didn't find such a specific one in connection with generalized operations.

(Note: the basics of the threadsafe stack example came from Anthony Williams' C++ Concurrency in Action.)

Thank you very much for any help in advance.

Regards, Zsolt

score 2 · Answer 1 · answered Jun 22 '17 at 00:17

Make ThreadSafeStack<U> a friend of ThreadSafeStack<T>:

template <typename T>
class ThreadsafeStack {
public:
    template <typename U>
    friend class ThreadSafeStack;
    //...
};

template <typename T>
template <typename U>
ThreadsafeStack<T>& ThreadsafeStack<T>::operator=(ThreadsafeStack<U> const& o)
{
  if ((void*) this == (void*) &o) {
    return *this;
  }

  std::lock(m, o.m);
  std::lock_guard<std::mutex> lock1(m, std::adopt_lock);
  std::lock_guard<std::mutex> lock2(o.m, std::adopt_lock);

  //...

  return *this;
}

Now ThreadSafeStack<T> can access the private members of ThreadSafeStack<U>. How you go about actually copying a std::stack<T> to a std::stack<U> is still up in the air though; there is no copy constructor or assignment operator that does that.

Upvoted for the observation at the end. It points to the real reason why this doesn't quite work. — Michaël Roy, Jun 22 '17 at 01:28

score 1 · Answer 2 · answered Jun 22 '17 at 00:33

If you had implemented the converting constructor (as suggested in you class declaration):

template <typename U>
ThreadsafeStack(ThreadsafeStack<U> const&);

You just need to implement the assingment operator declared as:

ThreadsafeStack<T>& operator=(ThreadsafeStack<T> o);

using the copy and swap idiom and let the converting constructor do the hard work

Michaël Roy · Answer 3 · 2017-06-22T01:24:11.793

Have you considered adding lock() and unlock() to your template class ?

I did something very similar before, the difference being that locking was the responsibility of the caller. Still, this may help you solve your access issue.

The idea behind my code was to find an easy way to make any struct/class thread-safe, in an non-intrusive way.

Exporting lock() and unlock() makes your template class compatible with std::lock_guard as well, which is nice.

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

template <typename T, typename _Mutex = std::mutex>
class Lockable : public T
{
public:
    typedef _Mutex Mutex;

private:
   Mutex mtx_;

public:    
    Lockable() = default;
    Lockable(T&& x) : T(std::move(x)), mtx_() {}

    Lockable(Lockable&& x) = default;

    Lockable(const T& t) : T(t), mtx_() {}

    template<typename M>
    Lockable(const Lockable<T,M>& x) : T(x), mtx_() { }

    //  needs few common operators, =, ==, !=
    T& operator = (const T& x) { return T::operator=(x); }

    template<typename M>
    Lockable& operator = (const Lockable<T,M>& x) 
    {
        T::operator=(static_cast<const T&>(x));
        return *this;
    } 

    void lock() const   { const_cast<Mutex&>(mtx_).lock(); }
    void unlock() const { const_cast<Mutex&>(mtx_).unlock(); }
};


int main()
{
    using LV = Lockable<std::vector<int>>;
    LV vec = std::vector<int>(10, 42);

    // this doesn't work, unfortunately 
    //Lockable<int> z{2};

    std::lock_guard<LV> lock{ vec };

    for (auto& x : vec)
    {
        std::cout << "x: " << x << '\n';
    }
    return 0;
}

A quite different approach, but a great idea. Thank you for pointing out. — laszlzso, Jun 22 '17 at 13:15

score 0 · Accepted Answer · answered Jun 22 '17 at 13:14

I saw multiple approaches and answers for the problem, and am very grateful for all your time. Since all of them are partial in a sense, but still useful, I venture to provide a comprehensive answer based on your suggestions.

Here is the upgraded class declaration (with friend declaration of other instantiations of the template) suggested by another answer:

template <typename T>
class ThreadsafeStack {
public:
    template <typename U>
    friend class ThreadsafeStack;
    //...
};

However, the definition of the generalized assignment operator was not complete. Here is the way I finished it:

template <typename T>
template <typename U>
ThreadsafeStack<T>& ThreadsafeStack<T>::operator=(ThreadsafeStack<U> const& o)
{
  if ((void*) this == (void*) &o) {
    return *this;
  }

  std::lock(m, o.m);
  std::lock_guard<std::mutex> lock1(m, std::adopt_lock);
  std::lock_guard<std::mutex> lock2(o.m, std::adopt_lock);

  data = std::stack<T>{};  // Note that the interface of std::stack doesn't provide a clear() member function.
  auto tmp = o.data;
  while (!tmp.empty()) {
    data.push((T) tmp.top());
    tmp.pop();
  }

  return *this;
}

Note that this implementation was similar to the copy-and-swap idiom but unfortunately, the swap part must have been performed in a longer way because of the potential differing types. (Of course, the checking and conversion of the types could be further improved, but is not essential for the actual problem.)

Finally, to be complete, here is the implementation of the rest of the members (copy constructors and the "normal" assignment operator):

template <typename T>
ThreadsafeStack<T>::ThreadsafeStack(ThreadsafeStack<T> const& o)
{
  std::lock_guard<std::mutex> lock(o.m);
  data = o.data;
}

template <typename T>
template <typename U>
ThreadsafeStack<T>::ThreadsafeStack(ThreadsafeStack<U> const& o)
{
  std::lock_guard<std::mutex> lock(o.m);

  data = std::stack<T>{};
  auto tmp = o.data;
  while (!tmp.empty()) {
    data.push((T) tmp.top());
    tmp.pop();
  }
}

template <typename T>
ThreadsafeStack<T>& ThreadsafeStack<T>::operator=(ThreadsafeStack<T> const& o)
{
  if ((void*) this == (void*) &o) {
    return *this;
  }

  std::lock(m, o.m);
  std::lock_guard<std::mutex> lock1(m, std::adopt_lock);
  std::lock_guard<std::mutex> lock2(o.m, std::adopt_lock);

  data = o.data;

  return *this;
}

C++ templates - Generalized operations and visibility of members between instantations

4 Answers4