Segmentation fault with shared_ptr of a vector element in C++

Viewed 2926

I'm new to C++ and I have a very hard time understanding relational mechanisms. I come from a PHP background with databases and ORMs and I fail to reproduce a very simple case. The program crashes with a segmentation fault in "atomic.h" line 49 { return __atomic_fetch_add(__mem, __val, __ATOMIC_ACQ_REL); }. The problem comes from the shared_ptr although I don't really understand how to use smart pointers (I've read hundreds of articles and SO questions about them but still very mysterious).

Without being able to make a working pointer on a repository element, my attempts to make one-to-many and many-to-one relationships between classes all failed miserably. I must be missing something, or maybe it's not at all the way we should do it in C++?

Note: I compile with GCC6.3 with C++17 using std only, not boost.

class Car : public enable_shared_from_this<Car> {
    //...
}

template <class Type>
class Repository<Type> {
    vector<Type> collection;
public:
    shared_ptr<Type> getNew() {
       Type newType;
       collection.push_back(newType);
       return shared_ptr<Type>(&collection.back());
       // I also tried this but program fails at this line although it compiles fine.
       // return collection.back().shared_from_this();
    }
}

class MainApp {
    Repository<Cars> cars;
    shared_ptr<Car> myCar;

public:
    MainApp() {
        myCar = cars.getNew();
    }
}

int main() {
    MainApp app; // OK - myCar points to the last car in cars repository
    return 0; // SIGEGV Segmentation fault
}
1 Answers

TLDN'R: You should either use vector of std::shared_ptr in repository or change getNewCar to return a reference.

I've change a little bit your code to make the output more verbose:

#include <memory>
#include <iostream>
#include <vector>

class Car
{
public:
    Car()
    {
        std::cout << "Car ctor @" << this << std::endl;
    }

    Car(const Car& car)
    {
        std::cout << "Car copy ctor @" << this << " from " << &car << std::endl;
    }

    Car& operator=(const Car& car)
    {
        std::cout << "Car assign operator @" << this << std::endl;
    }

    ~Car()
    {
        std::cout << "Car dtor @" << this << std::endl;
    }
};

class Repository
{
    std::vector<Car> collection;
public:
    std::shared_ptr<Car> getNewCar()
    {
        Car newType;
        collection.push_back(newType);
        return std::shared_ptr<Car>(&collection.back());
    }
};

int main()
{
    Repository rep;
    std::shared_ptr<Car> myCar = rep.getNewCar();
    return 0;
}

The code results with:

Car ctor @0x7ffe35557217
Car copy ctor @0x25b6030 from 0x7ffe35557217
Car dtor @0x7ffe35557217
Car dtor @0x25b6030
Car dtor @0x25b6030

The problem you are facing is the fact that your code executes Car destructor twice on the same object and this can lead to a number of odd behaviors and you are lucky that in your case it is a Segmentation fault.

The std::vector you are using in a repository is responsible to delete all its object when it is destroyed. The last 'std::shared_ptr' that points to some object also is responsible for doing so.

In line:

return shared_ptr<Type>(&collection.back());

You take the address of a object that currently belongs to std::vector and creates the std::shared_ptr with a pointer. This std::shared_ptr thinks that it is the first smart pointer that keeps track of the object. So when it is destroyed (or the last of its copy if you make some) this objects destructor will be called.

So if you believe that the Car will live longer than the repository that it is from:

class Repository
{
    std::vector<std::shared_ptr<Car>> collection;
public:
    std::shared_ptr<Car> getNewCar()
    {
        collection.push_back(std::shared_ptr<Car>(new Car()));
        return collection.back();
    }
};

If you are sure that the Car will be destroyed before the Repository is destroyed:

class Repository
{
    std::vector<Car> collection;
public:
    Car& getNewCar()
    {
        Car newCar;
        collection.push_back(newCar);
        return collection.back();
    }
};
Related