A typical (blocking) implementation of a thread-safe queue goes something like:

template <class T> 
class ts_queue
{
  std::queue<T> m_data;
  mutable std::mutex m_mtx;
  std::condition_variable m_cv;

public:
  /*
    Implementation
  */
};

The very inclusion of mutex and condition_variable in the member-list, makes this class non-moveable and non-copyable. Since moving is a reasonable requirement (you don't want classes containing a thread-safe queue to be pinned down to the place of creation) there's two popular approaches to satisfy it:

1. Use smart_ptr<ts_queue<T>> when you want the instance to be moveable. So, the class remains non-moveable but can be used in way it doesn't cause trouble if the need to move arises.
2. Put the queue's non-moveable state in a smart pointer. Essentially remove the need of the container to be used as a pointer by hiding the allocation inside the class:

   template <class T> 
   class ts_queue
   {
     struct state {
       std::queue<T> m_data;
       mutable std::mutex m_mtx;
       std::condition_variable m_cv;
     };
     // This guy can now be moved.
     std::unique_ptr<state> m_state;
   
   public:
     /*
       Implementation
     */
   };
   

My question is: is it best practice to make such a class move-able? Given that approach (1) can handle most of our needs, what should a library author opt for?

To extend a bit the background on this question, I see that in the C concurrency book by A.Williams the author provides exclusively "non moveable" implementations of blocking (having a mutex and condition variable) thread-safe queues. Interestingly, one of the implementations is "copyable"; same applies for boost queue - copyable but not moveable (it has a user-declared destructor and no move method implemented).

I can see how "moving while someone might be waiting on the queue" can be problematic or tough to handle. The question is, is it worth the trouble, is there an accepted best practice?

CodePudding user response：

Why do you say the queue is not movable? If you lock the queue and the other queue then it's perfectly fine to move the m_data.

The only problem is that you run into the dining philosophers problem if you have multiple queues and try to move in a cyclic pattern. Avoid any cycles and you avoid that.

CodePudding user response：

(2) is the choice that makes the queue more useful. Immovable objects in C are generally much more difficult to work with at construction. One can construct a prvalue and initialize the eventual object with it. But you can't do this if there's any reference in between... So you can do this if you have the eventual object directly, or it's in an aggregate, but not if it's in a pair or map. The rules just become a lot more obscure, and you have to workaround them with emplace-like APIs.

The downside of (2) is the indirection is required even if the user didn't need the movability. That could be a consideration.

For a general library, you would probably then prefer (2).