How to deal with noncopyable objects when inserting to containers in C++

Question

I'm looking for the best-practice of dealing with non-copyable objects.

I have a mutex class, that obviously should not be copyable. I added a private copy constructor to enforce that.

That broke the code - some places simply needed to be fixed, but I have a generic problem where a class, using the mutex either as a data member, or by inheritance, is being inserted into a container.

This is usually happening during the container initialization, so the mutex is not initialized yet, and is therefore ok, but without a copy constructor it does not work. Changing the containers to contain pointers is not acceptable.

Any advise?

Answer 1

If an object is non-copyable then there is usually a good reason. And if there's a good reason then you shouldnt subvert that by putting it into a container that attempts to copy it.

Answer 2

STL containers rely heavily on their contents being copyable, so either make them copyable or do not put them into container.

Answer 3

If they are non-copyable, the container has to store (smart) pointers to those objects, or reference wrappers, etc, although with C++0x, noncopyable objects can still be movable (like boost threads), so that they can be stored in containers as-is.

to give examples: reference wrapper (aka boost::ref, a pointer under the hood)

#include <vector>
#include <tr1/functional>
struct Noncopy {
private:
        Noncopy(const Noncopy&) {}
public:
        Noncopy() {}
};
int main()
{
        std::vector<std::tr1::reference_wrapper<Noncopy> > v;
        Noncopy m;
        v.push_back(std::tr1::reference_wrapper<Noncopy>(m));
}

C++0x, tested with gcc:

#include <vector>
struct Movable {
private:
        Movable(const Movable&) = delete;
public:
        Movable() {}
        Movable(Movable&&) {}
};
int main()
{
        std::vector<Movable> v;
        Movable m;
        v.emplace_back(std::move(m));
}

EDIT: Nevermind, C++0x FCD says, under 30.4.1/3,

A Mutex type shall not be copyable nor movable.

So you're better off with pointers to them. Smart or otherwise wrapped as necessary.

Answer 4

The best option is to use pointers or some type of wrapper class that uses pointers under the hood. That would allow these to be sanely copied, and actually do what a copy would be expected to do (share the mutex).

But, since you said no pointers, there is one other option. It sounds like Mutexes are "sometimes copyable" perhaps you should write a copy constructor and an assignment operator and have those throw an exception if a mutex is ever copied after it has been initialized. The down side is there's no way to know you're doing it wrong until runtime.

Answer 5

Three solutions here:

1. Use Pointers - The quick fix is to make it a container of pointers - e.g. a shared_ptr.

That would be the "good" solution if your objects are truly noncopyable, and using other containers is not possible.

2. Other containers - Alternatively, you could use non-copying containers (that use in-place-construction), however they aren't very common and largely incompatible with STL. (I've tried for a while, but it's simply no good)

That would be the "god" solution if your objects are truly noncopyable, and using pointers is not possible.

[edit] With C++13, std::vector allows inplace construction (emplace_back), and can be used for noncopyable objects that do implement move semantics. [/edit]

3. Fix your copyability - If your class is copyable as such, and the mutex is not, you "simply" need to fix the copy constructor and assignment operator.

Writing them is a pain, since you usually have to copy & assign all members except the mutex, but that can often be simplified by:

template <typename TNonCopyable>
struct NeverCopy : public T 
{
    NeverCopy() {}
    NeverCopy(T const & rhs) {}

    NeverCopy<T> & operator=(T const & rhs) { return *this; }
}

And changing you mutex member to

NeverCopy<Mutex> m_mutex;

Unfortunately, using that template you lose special constructors of Mutex.

[edit] Warning: "Fixing" the Copy CTor/asignment often requires you to lock the right hand side on copy construct, and lock both sides on assignment. Unfortunately, there is no way to override the copy ctor/assignment and call the default implementation, so the NeverCopy trick might not work for you without external locking. (There are some other workarounds with their own limitations.)

Answer 6

Using a mutex in a class does not necessarily mean that the class has to be non-copyable. You can (almost) always implement it like this:

C::C (C const & c)
// No ctor-initializer here.
{
  MutexLock guard (c.mutex);

  // Do the copy-construction here.
  x = c.x;
}

While this makes it somewhat possible to copy classes with mutexes, you probably should not do it. Chances are that your design will be better without per-instance mutex.

Answer 7

Use smart pointers like boost::shared_ptr or use another containers, like boost::intrusive. Both will require to modify your code, through.

Answer 8

There is no real answer to the question given how you've framed it. There is no way to do what you want. The actual answer is for the container to contain pointers, and you've said that isn't OK for some unspecified reason.

Some have talked about things being movable and using C++0x in which containers often require their elements to be movable, but do not require them to be copyable. I think this is a poor solution as well because I suspect that mutexes should not be moved while they are held, and this makes it effectively impossible to move them.

So, the only real remaining answer is to point at the mutexes. Use ::std::tr1::shared_ptr (in #include <tr1/memory>) or ::boost::shared_ptr to point at the mutexes. This requires changing the definitions of the classes that have the mutexes inside them, but it sounds like you're doing that anyway.

Answer 9

Using c++11 on Ubuntu 14.04 (which includes emplace_back), I've gotten this to work.

I found that emplace_back worked fine, but erase (and probably insert) didn't work because, when the vector was shuffling the elements along to fill in the gap, it mas using:

 *previous = *current;

I found the trick was allowing move assignment in my resource class:

  Watch& operator=(Watch &&other);

This is my inotify_watch class, which can live in a std::vector:

class Watch {
private:
  int inotify_handle = 0;
  int handle = -1;
  // Erases all knowledge of our resources
  void erase() {
    inotify_handle = 0;
    handle = -1;
  }
public:
  Watch(int inotify_handle, const char *path, uint32_t mask)
      : inotify_handle(inotify_handle),
        handle(inotify_add_watch(inotify_handle, path, mask)) {
    if (handle == -1)
      throw std::system_error(errno, std::system_category());
  }
  Watch(const Watch& other) = delete; // Can't copy it, it's a real resource
  // Move is fine
  Watch(Watch &&other)
      : inotify_handle(other.inotify_handle), handle(other.handle) {
    other.erase(); // Make the other one forget about our resources, so that
                   // when the destructor is called, it won't try to free them,
                   // as we own them now
  } 
  // Move assignment is fine
  Watch &operator=(Watch &&other) {
    inotify_handle = other.inotify_handle;
    handle = other.handle;
    other.erase(); // Make the other one forget about our resources, so that
                   // when the destructor is called, it won't try to free them,
                   // as we own them now
    return *this;
  }
  bool operator ==(const Watch& other) {
    return (inotify_handle == other.inotify_handle) && (handle == other.handle);
  }
  ~Watch() {
    if (handle != -1) {
      int result = inotify_rm_watch(inotify_handle, handle);
      if (result == -1)
        throw std::system_error(errno, std::system_category());
    }
  }
};

Answer 10

std::vector can not store non-copyable objects (due to resize) thus you can't store objects of type Foo:

struct Foo {
   std::mutex mutex;
   ...
};

One way around this is to use std::unique_ptr:

struct Foo {
   std::unique_ptr<std::mutex> pmutex;
   Foo() : pmutex{std::make_unique<std::mutex>()} {}
   ...
};

Another option is to use a std::deque which can hold non-copyable objects (like instances of the first version of Foo above. Typically use emplace_back method to construct objects "in place" to add elements -- no copies happen. For example, objects of type Foo here do not need to be copiable:

struct FooPool {
    std::deque<Foo> objects;
    ObjectPool(std::initializer_list<T> argList) {
        for (auto&& arg : argList)
             objects.emplace_back(arg);
    ...
};