Get error when I push element onto vector

Question

I get a is implicitly deleted because the default definition would be ill-formed error with below code when I want to add the element to the vector with container.push_back(tmp);, why is this? My code:

#include <vector>
#include <mutex>
class A {
    public:
    A(){}
    ~A(){}
};
class my_t {
    private:
        bool busy;
        bool alive;
        std::mutex bmtx;
        std::mutex amtx;
    public:
        my_t(){std::lock_guard<std::mutex>lock(bmtx);busy=false;}
        ~my_t(){}
        A* conn; 
        void take(void) {std::lock_guard<std::mutex>lock(bmtx);busy=true; }
        void give(void) {std::lock_guard<std::mutex>lock(bmtx);busy=false; }
        bool busy_get(void) {return busy;}
        void set_online(void){ std::lock_guard<std::mutex>lock(amtx); alive=true; }
        void set_OFFLINE(void) {std::lock_guard<std::mutex>lock(amtx); alive=false; }
        bool alive_get(void) {return alive;}

};

class app {
    private:
        std::vector<my_t> container;
    public: 
        app();
        ~app();
};
app::app() {
    my_t tmp;
    tmp.conn = new A();
    if (tmp.conn)
        tmp.set_online();
    container.push_back(tmp);


}
int main(void) {}

or on Coliru: https://coliru.stacked-crooked.com/a/11625c77383df80c

When you push into the `container`, you try to **copy** a `my_t` object. This would invoke the (default, compiler-provided) copy constructor, but this can't exist because it would copy all the members, and [`std::mutex` has a `delete`d copy constructor.](https://en.cppreference.com/w/cpp/thread/mutex/mutex). — BoBTFish, Oct 06 '18 at 23:16
@UlrichEckhardt To be honest I haven't tried to compile it, but it looks like it will compile and exhibit the issue. It's not strictly minimal, but it's not too huge either, only a few extra functions, which are one-liners. — BoBTFish, Oct 06 '18 at 23:26
Not being a MCVE, it makes your question off-topic here. There are good reasons for that, please follow the provided link! — Ulrich Eckhardt, Oct 06 '18 at 23:27
Sidenote: having `A* conn;` in the `my_t` class suggests you should keep an eye out for the [Rules of Three and Five](https://en.cppreference.com/w/cpp/language/rule_of_three). to ensure things are copied correctly once you get the `mutex` problem sorted out. — user4581301, Oct 06 '18 at 23:36
@UlrichEckhardt the provided example is much minimized already. I wasn't EXACTLY sure what in my program caused the issue & didn't want to omit anything! In order to get the right kind of help from you guys!I'll do better going forward though! Thanks to all involved! — stdcerr, Oct 07 '18 at 00:01
"I wasn't EXACTLY sure what in my program caused the issue" -- that's why you are expected to extract a MCVE before asking here. — Ulrich Eckhardt, Oct 07 '18 at 07:05

score 4 · Answer 1 · answered Oct 06 '18 at 23:20

You cannot have a vector of your my_t objects, because the my_t class contains std::mutexes:

 std::mutex bmtx;
 std::mutex amtx;

Mutexes have a deleted copy constructor. There's no such thing as copy-constructing a mutex.

Since your my_t class also does not have a copy constructor, this automatically deletes my_t's copy constructor as well.

std::vector's value class must have, at the very least, a copy constructor and an assign operator. This is because a vector must be able to reallocate its contents, in order to accomodate its growth. This means that the elements in the vector must be copyable/assignable.

If you explicitly implement a copy constructor and an assignment operator for your my_t class, you'll be able to make a vector out of them. But then you will need to figure out what it means to copy-construct, and assign, to your my_t class...

The most you can do, with your class as is, is to have a vector of pointers to my_t instances. Perhaps using std::unique_ptr or std::shared_ptr.

score 3 · Answer 2 · answered Oct 06 '18 at 23:21

In this line:

container.push_back(tmp);

you try to copy a my_t object. This would invoke the (default, compiler-provided) copy constructor, but this can't exist. It can't exist because it would copy all the members one-by-one, and std::mutex has a deleted copy constructor..

It doesn't really make sense for a mutex to be copy-able. They don't have value semantics, and they only make sense when exist alongside an actual value.

So create your own copy constructor for my_t, which copies all the members that it can, and just default constructs the others.

You almost certainly also want to deep copy the conn member, and make it a std::unique_ptr<A> as well, to make sure it gets deleted properly when the object gets destructed.

Get error when I push element onto vector

2 Answers2