Not sure what's wrong -- Program Segfaults on two occasions

Question

I'm using SDL to create a pretty simple Pong game. For the collision detection, I have a class called DetectCollision which looks like this:

class DetectCollision {
    public:
        std::vector<Object*> objects;
        int numOfObjects;

        DetectCollision();
        ~DetectCollision();

        void takeObjs(Object &);
        void handleCollision();
};

So what happens is the takeObjs(Object &) function takes an object of the Object class and stores it in a vector. takeObjs(Object &) looks like this:

void DetectCollision::takeObjs(Object &obj1){
    objects.push_back(&obj1);
}

Everything works up to this point. I'm able to access the vector "objects" to detect all the collisions and it works fine, but the problem comes when I am trying to delete the vector. If I'm not mistaken, it's a vector of pointers. So what I did to delete it is in the class destructor:

DetectCollision::~DetectCollision(){
    for (unsigned int i = 0; i < objects.size(); ++i){
        delete objects[i];
    }
    objects.clear();
}

According to the Code::Blocks IDE using a GCC compiler, it segfaults on the "delete objects[i];" line. It also segfaults on another part of the program to attempts to SDL_FreeSurface() the skins held by the class, but I think I can fix that one easily enough. This vector is the main issue. If you need to look at the full source code to help me learn how to fix the issue, I can provide that. I am very grateful for any assistance. Thanks and DFTBA!

score 0 · Answer 1 · answered Mar 23 '13 at 17:31

0

One potential problem is that DetectCollision violates the Rule of Three. If an instance were copied, the copy would get a vector of the same Object pointers, eventually resulting in double deletes.

answered Mar 23 '13 at 17:31

NPE

486,780
108
951
1,012

score 0 · Accepted Answer · edited May 23 '17 at 12:04

Your app segfaults because the objects no longer exist when the destructor is called.

The takeObjs method receives a reference to an object, and then it stores a pointer to that object inside the vector. That object is defined somewhere else. When that object goes out of scope, it is automatically destroyed.

By the time you get to the destructor, the object is destroyed. The app segfaults because you are trying to destroy it again.

You should read about the scope of objects in C++ (read the answer to this question) and you should also read about Object destruction in C++.

EDIT: Added short example to illustrate crash

#include <iostream>
#include <vector>
using namespace std;

class Object {
public:
    int a;
    int b;

    Object(int a, int b)
    {
        this->a=a;
        this->b=b;
    }
};

class Test
{
    std::vector<Object*> objects;
public:
    Test(){}

    void Add(Object &obj)
    {
        objects.push_back(&obj);
    }

    void Print()
    {
        for(unsigned int i=0;i<objects.size();i++)
        {
            cout<<objects[i]->a<<" "<<objects[i]->b<<endl;
        }
    }

    ~Test()
    {
        for (unsigned int i = 0; i < objects.size(); ++i){
            delete objects[i];
        }
        objects.clear();
    }
};

void AddNewObjects(Test &t)
{
    Object x(1,2);
    Object y(3,4);
    t.Add(x);
    t.Add(y);
    // you can access your objects here
    t.Print();
}

int _tmain(int argc, _TCHAR* argv[])
{
    Test t;
    AddNewObjects(t);
    // but if you try to access the objects here, you get a crash
    // because the objects were destroyed when exiting "AddNewObjects"
    t.Print();
    return 0;
    // your destructor tries to access the objects here (in order to destroy them)
    // and that's why it crashes
}

Here is one solution you can use in order to fix the problem:

#include <iostream>
#include <vector>
using namespace std;

class Object {
public:
    int a;
    int b;

    Object(int a, int b)
    {
        this->a=a;
        this->b=b;
    }
};

class Test
{
    std::vector<Object*> objects;
public:
    Test(){}

    void Add(Object *pObj)
    {
        objects.push_back(pObj);
    }

    void Print()
    {
        for(unsigned int i=0;i<objects.size();i++)
        {
            cout<<objects[i]->a<<" "<<objects[i]->b<<endl;
        }
    }

    ~Test()
    {
        for (unsigned int i = 0; i < objects.size(); ++i){
            delete objects[i];
        }
        objects.clear();
    }
};

void AddNewObjects(Test &t)
{
    Object* x = new Object(1,2);
    Object* y = new Object(3,4);
    t.Add(x);
    t.Add(y);
    // you can access your objects here
    t.Print();
}

int _tmain(int argc, _TCHAR* argv[])
{
    Test t;
    AddNewObjects(t);
    // you can also access the objects here
    // because they are not destroyed anymore when exiting "AddNewObjects"
    t.Print();
    return 0;
}

If the object is destroyed after leaving the scope of takeObjs, then how can the rest of the program access the vector of objects and use its elements? When does it destroy? — Jpon9, Mar 23 '13 at 17:48
because it accesses stack locations, did you allocate the objects before adding them ? — Alon, Mar 23 '13 at 17:50
The object is not destroyed after leaving the scope of `takeObjs`. The object is destroyed after your program finishes accessing the elements, but before your destructor starts to run. — Ove, Mar 23 '13 at 17:50
@Jpon9 I've added a short example to illustrate why you get a segfault. — Ove, Mar 23 '13 at 18:00
Thanks a ton, Ove! Your solution worked and I got it working small-scale. In my game, though, it still segfaults on closing for several reasons I can't figure out. CodeBlocks is also putting a red bar on a libstdc++-6!_ZN9__gnu_cx18__exchange_and_add_EPVii() function, which looks like gibberish to me. It also points at the empty destructor of the Object class saying it causes a segfault. But all of that aside, it also returns -1073741819. If anyone wants to help me further, I would appreciate it. I can provide other contact info and source code if anyone's willing... I'm really stuck here. — Jpon9, Mar 25 '13 at 12:26
You might have some similar issues elsewhere in your source code. You should read about the scope of objects in C++ ( [read the answer to this question](http://stackoverflow.com/q/10080935/80003) ) and you should also read about [Object destruction in C++](http://stackoverflow.com/q/6403055/80003). — Ove, Mar 25 '13 at 13:12

score 0 · Answer 3 · answered Mar 23 '13 at 17:33

0

You probably have a double delete because the Objects passed to your DetectCollision class are deleted elsewhere, probably because they go out of scope.

You have to think about who is going to "own" the objects. Only the owner should delete them.

answered Mar 23 '13 at 17:33

Geier

894
7
16

Not sure what's wrong -- Program Segfaults on two occasions

3 Answers3