Search code examples
c++pointersvectorstrategy-pattern

C++ Why emplacing object in vector segfaults?


I want to create a vector of "Act" objects that contain pointers to either "Eat" or "Drink" dynamically allocated objects. The new objects are being emplaced like so:

action_vector.emplace_back(Act::BehaviorType::eat);

However, it is seg-faulting and I can't figure out why. I thought that emplace_back would implicitly call the move constructor, not the destructor, but for some reason it is, which (I think) is what is screwing everything up.

Is there any way to successfully create a vector of such objects?

Here is the rest of the code along with its output. Sorry if it's a little verbose, but basically it's just a strategy pattern.

#include <iostream>
#include <vector>

class IBehavior
{
public:
    IBehavior() = default;
    virtual ~IBehavior() = default;
    virtual void execute() = 0;
};

class Drink : public IBehavior
{
public:
    Drink(): IBehavior() {}
    ~Drink() {}
    void execute() { std::cout << "Drinking" << std::endl; }
};

class Eat : public IBehavior
{
public:
    Eat(): IBehavior() {}
    ~Eat() {}
    void execute() { std::cout << "Eating" << std::endl; }
};


class Act
{
    IBehavior * b;

public:

    enum class BehaviorType { eat = 0, drink = 1 };

    Act() = default;
    ~Act()
    {
        std::cout << "Calling the destructor" << std::endl;
        delete b;
    }
    Act(BehaviorType b_type) { SetBehavior(b_type); }

    Act(Act&& act)
    {
        std::cout << "Calling the move constructor" << std::endl;
        this->b = act.b;
    }


    void SetBehavior(BehaviorType b_type)
    {
        if(b_type == BehaviorType::eat) b = new Eat();
        if(b_type == BehaviorType::drink) b = new Drink();
    }

    void execute() { b->execute(); }
};


int main(int argc, char * argv[])
{
    std::vector<Act> action_vector;

    for(int i = 0; i < 10; ++i)
    {
        action_vector.emplace_back(Act::BehaviorType::eat);
        action_vector[i].execute();
    }

    return 0;
}

output:

Eating
Calling the move constructor
Calling the destructor
Eating
Calling the move constructor
Calling the move constructor
Calling the destructor
Calling the destructor
Segmentation fault: 11

Solution

  • Your move constructor copies b, and destructor deletes b, so if you move construct an instance then the same pointer value will be deleted twice which has undefined behaviour.

    General solution: Use a smart pointer.


    Another bug: Default constructor leaves b uninitialised. When a default constructed object is destroyed, the uninitialised pointer is deleted and behaviour is undefined. Smart pointer fixes this also.