Simple class, delete exception

[mikazo]

I apologize if this is a very simple error that has slipped my mind, but does anyone see anything wrong with this class?

class Cell {
public:
	Cell() {
		o1 = new GameObject();
		o2 = new GameObject();
		o3 = new GameObject();
		o4 = new GameObject();
	}
	~Cell() {
		delete o1;
		delete o2;
		delete o3;
		delete o4;
	}
private:
	GameObject * o1;
	GameObject * o2;
	GameObject * o3;
	GameObject * o4;
};

When the program is exited and the destructor runs, I get this error:

Unhandled exception at 0x68b631ea (msvcr90d.dll) in Erg.exe: 0xC0000005: Access violation reading location 0xfeeefee2.

I realize this means I am trying to delete something that is not there. Should I be checking that the allocation ran properly? Is the constructor somehow not running?

I'm running Visual Studio 2008 on Windows Vista Home Premium.

Thanks for any help and sorry again if it's something small and stupid.

[Skizmo]

You code looks ok, and the default constructor and destructor are always called. Did you use a debugger to confirm that the value of 'o1' to 'o4' actually is 0xfeeefee2 ? If all the 4 variables contain valid pointers, maybe there is a problem with the GameObject you are trying to delete.

[streamer]

You don't have any problem.

However you should write

if(o1)
{
delete o1;
o1 = NULL;
}

Possibly you are not deleting Cell class properly,

If you have ie defined some variable globally, as
Cell g_MYCELLS[10][10];

destructor is called on the end of program exit if I know well.

You should put that in class somewhere and call the delete accordingly when program is exiting

class Game
{
Cell *pCells;

~Game()
{
if( pCells ) delete pCells
}
}

[GCDEF]

Run the debugger and see what line is causing the error. Is it possible it's your GameObject destructor that's crashing?

[GCDEF]

You don't have any problem.

However you should write

if(o1)
{
delete o1;
o1 = NULL;
}

Possibly you are not deleting Cell class properly,

If you have ie defined some variable globally, as
Cell g_MYCELLS[10][10];

destructor is called on the end of program exit if I know well.

You should put that in class somewhere and call the delete accordingly when program is exiting

class Game
{
Cell *pCells;

~Game()
{
if( pCells ) delete pCells
}
}

He's unconditionally setting them in the constructor.
Calling delete on a null pointer is okay.
There's no point in setting their value to NULL in a destructor because they won't be accessible again anyway.

[laserlight]

Did you really forget to implement or disable the copy constructor and copy assignment operator? Maybe the code that uses this class performs copying, leading to double deletion since the compiler generated copy constructor and copy assignment operator perform shallow copying.

[Paul McKenzie]

I apologize if this is a very simple error that has slipped my mind, but does anyone see anything wrong with this class?

This simple 2 line program messes up your Cell class:

int main()
{
   Cell a;
   Cell b = a;
}

The problem is that your class will go haywire if it's copied or assigned. You either have to disable these operations, or code a user-defined copy constructor and assignment operator.

Before coding any class, you must be aware of these issues of copying and assignment, and handle them accordingly. Don't leave it as an afterthought.

Regards,

Paul McKenzie

[TheCPUWizard]

You either have to disable these operations, or code a user-defined copy constructor and assignment operator.

Or not do silly allocation of variables using naked/raw pointers in the first place.

[GCDEF]

Or not do silly allocation of variables using naked/raw pointers in the first place.

Assuming this isn't a hypothetical example, that's the obvious answer.

[mikazo]

Ok I need to rescue my credibility here. I know exactly what copy and assignment constructors are and how they work and why you need them. I'm not here for a programming lesson. I'm in second year computer science and would hope that all second year computer science students know what copy and assignment constructors are by now.

I have just just started this project (hence the very scantily-clad classes). In my main(), the only thing I do is create an array of Cell classes, then delete them. Nothing else happens yet. The GameObject class also has barely any code in it.

All I want to know is, why can't I initialize a Cell containing a private pointer to a GameObject, then delete the Cell and the GameObject?

[GCDEF]

Have you taken the suggestion of a couple of us and used the debugger? What did it tell you?

[streamer]

Ok I need to rescue my credibility here. I know exactly what copy and assignment constructors are and how they work and why you need them. I'm not here for a programming lesson. I'm in second year computer science and would hope that all second year computer science students know what copy and assignment constructors are by now.

I have just just started this project (hence the very scantily-clad classes). In my main(), the only thing I do is create an array of Cell classes, then delete them. Nothing else happens yet. The GameObject class also has barely any code in it.

All I want to know is, why can't I initialize a Cell containing a private pointer to a GameObject, then delete the Cell and the GameObject?

The problem is not in code you had posted. Problem is somewhere else in the program. Without some other code we can't help. Just tip what MAY be the problem.

[Paul McKenzie]

Ok I need to rescue my credibility here. I know exactly what copy and assignment constructors are and how they work and why you need them. I'm not here for a programming lesson.

The only code you posted shows a class that lacks copy constructor and assignment operator. We are left to conclude that there is a high possibility that this is what's causing the problem. If you did have all of those things, then post them. If you did not have those things, regardless if you know what they are or not, then the points that myself and others are making are valid -- a lack of a copy constructor and assignment operator will cause the issues you stated.

In my main(),

Please post what you are doing in main(). Do not describe it -- post exactly what it consists of.

The GameObject class also has barely any code in it.

Programming is an exact science -- there is no such thing as "barely". Post exactly what GameObject is.

All I want to know is, why can't I initialize a Cell containing a private pointer to a GameObject, then delete the Cell and the GameObject?

Provide the missing code, and you will get an answer. Don't provide the code, and we can keep guessing from here to who knows...

Regards,

Paul McKenzie

[mikazo]

class GameObject {
public:
	GameObject() {
	}
	~GameObject() {
	}
private:
	int a;
};

class Cell {
public:
	Cell() {
		o1 = new GameObject();
		o2 = new GameObject();
		o3 = new GameObject();
		o4 = new GameObject();
	}
	~Cell() {
		delete o1;
		delete o2;
		delete o3;
		delete o4;
	}
private:
	GameObject * o1;
	GameObject * o2;
	GameObject * o3;
	GameObject * o4;
};

Cell * grid[50][50];
for (int i=0; i<50; i++) {
	for (int j=0; j<50; j++)
		grid[i][j] = new Cell();
}
for (int i=0; i<50; i++) {
	for (int j=0; j<50; j++)
		delete grid[i][j];
}

Did you really forget to implement or disable the copy constructor and copy assignment operator?

My apologies for taking offence, but the way it was worded implied that I had no idea what I was doing.

I used the debugger, and apparently in the line _ASSERTE(_BLOCK_TYPE_IS_VALID(pHead->nBlockUse)); in dbgdel.cpp pHead causes the problem, having an access violation/null value.

Let me know if anything else is needed.

[Paul McKenzie]

I am running the following program in Visual Studio 2005.

class GameObject {
public:
    GameObject() {
    }
    ~GameObject() {
    }
private:
    int a;
};

class Cell {
public:
    Cell() {
        o1 = new GameObject();
        o2 = new GameObject();
        o3 = new GameObject();
        o4 = new GameObject();
    }
    ~Cell() {
        delete o1;
        delete o2;
        delete o3;
        delete o4;
    }
private:
    GameObject * o1;
    GameObject * o2;
    GameObject * o3;
    GameObject * o4;
};

int main()
{
    typedef Cell *CELLPTR;
    CELLPTR grid[50][50];
    for (int i=0; i<50; i++) {
        for (int j=0; j<50; j++)
            grid[i][j] = new Cell();
    }

    for (int i=0; i<50; i++) {
        for (int j=0; j<50; j++)
            delete grid[i][j];
    }
}

There were no errors when running the program.

Regards,

Paul McKenzie