views:

201

answers:

8

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?

+4  A: 

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.

Visage
This is a good general piece of advice, so I'm not downvoting you. But it's also a non-answer, so I'm sorely tempted to.
Omnifarious
In deed, it is an answer. What DBBD has asked is wrong. Visage tells him what is wrong: claiming objects are non-copyable but using them in a container that copies them. It is non-sense. Moreover DBBD doesn't even tell why using pointers is not acceptable, whereas it IS the way it should be, (most likely).
Stephane Rolland
@Omnifarious: Some people just ask wrong questions. Visage is IMHO right.
wilx
@wilx - I think that an answer to a 'wrong question' is an explanation, not a simple admonition.
Omnifarious
+1 Omnifarious. This is a non-answer. It doesn't even help the OP figure out *what his problem is*, it just tells him "you are stupid." ---- Even if a mutex is noncopyable, why should an object aggregating the mutex be? (*Answer: because it's the default*). In my experience, the solution here is usually to fix copy + assignment, rather than to "don't copy".
peterchen
+2  A: 

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

+2  A: 

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.

Cubbi
For most C++0x containers, the objects don't even have to be movable; `emplace` can construct them in place. They do need to be movable for `vector` and `deque`, since these need to reallocate memory as they grow.
Mike Seymour
Also, the C++0x accepted way of removing the copy constructor is `Moveable(const Movable `.
Omnifarious
This is really interesting, but I suspect that if a mutex is moved while it's held, something really bad might happen with some implementations.
Omnifarious
@Omnifarious: Why? Moving an OS object like a mutex simply involves copying some pointers. The OS doesn't even know what's going on. Also, rvalue references do not need to be const- it's in their definition that the point of them is to modify them.
DeadMG
@DeadMG - So, what about all the `this` pointers that are referring to the original object's location? And how about if the mutex is implemented so that its address is how the OS identifies it?
Omnifarious
@Omnifarious: thanks, edited to look more C++0x-ish.. and then looked up C++0x FCD and found out that "A Mutex type shall not be copyablenor movable." (30.4.1/3). oops
Cubbi
+1  A: 

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.

SoapBox
+1  A: 

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.

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.)

peterchen
A: 

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.

wilx
A: 

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

blaze
+1  A: 

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.

Omnifarious
Moving an object like a mutex is a question of moving some OS-provided pointers, nothing more. You can't move an actual mutex, but it's trivial to move the OS-provided handle to it.
DeadMG
@DeadMG - If the mutex is held or contended, that likely means there are `this` pointers pointing at the original object's location. Also, it's possible that the OS identifies the mutex by its memory address.
Omnifarious