why is deleting this object causing problems?

▲
2
▼

Instantiation:

weapons.push_back(new Pistol());
weapons.push_back(new Rifle());
weapons.push_back(new Shotgun());

destructor, when the first delete happens, the code breaks. This happens when I close the program.

Brain::~Brain()
{
    for (unsigned int i = 0; i < weapons.size(); i++)
    {
        delete weapons[i]; // this is where the code breaks
    }
}

I get a warning:

Unhandled exception at 0x0096371f in D3D10DEMO.exe: 0xC0000005: Access violation reading location   0x000002ce.

weapons is this:

weapons(vector<Gun*>())

Edit - I have deleted much of the code from this question but I have also cut down my program so as to reproduce the problem in a much smaller solution here:

http://dl.dropbox.com/u/13519335/D3D10DEMO_0.25.MinRep.zip

c++
winapi
asked on Stack Overflow Nov 24, 2011 by SirYakalot • edited Dec 5, 2011 by SirYakalot

2 Answers

▲
7
▼

You haven't defined virtual destructors for your weapon classes. http://www.parashift.com/c++-faq-lite/virtual-functions.html#faq-20.7

answered on Stack Overflow Nov 24, 2011 by FeatureShock
▲
4
▼

You problem is the definition of 

class Brain : public Entity
{
private:
    std::vector<Gun*> weapons;

and the ownership of Gun* by Brain object.

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

If a Brain is copy constructed the delete will be called multiple times deleting the same Gun from different weapons vector. And a Brain temporary is created when you add your Agents (Agent being a derived class of Brain) like so in main function.

int main()
{
    Level* level;
std::vector<Agent> agents;

level = new Level(agents);

for (int i = 0; i < 1; i++)
    {
        //TODO - health is a pointless parameter here
        agents.push_back(Agent(100, *level, agents, level->Pickups(), D3DXCOLOR(1.0F, 0.4f, 0.4f, 1.0f)));
    }

delete level;

}

 If you implement a copy constructor for Brain that clones the Gun* vector you should be ok. Alternative you should use shared_ptr<Gun> in your vector so that you don't have to delete them at all.

To summarize your problem boils down to

class Foo{};

class Bar
{
public:
    Bar()
    {
        mFooVec.push_back( new Foo() );
        mFooVec.push_back( new Foo() );
    }

    ~Bar()
    {
        for( unsigned int i = 0;i < mFooVec.size(); ++i )
        {
            delete mFooVec[i];
        }
    }

    std::vector<Foo*> mFooVec;
};

int main()
{
    Bar x;
    Bar y = x;

    return 0;
}

Here both Bar x and y have the same two Foo* in their mFooVec

answered on Stack Overflow Dec 5, 2011 by parapura rajkumar • edited Dec 6, 2011 by parapura rajkumar

User contributions licensed under CC BY-SA 3.0