← Back to index

Memory management (cleanup) in C++

Den V
#10149
Hello, All!

I am working on some simple graphics and ran into a problem with memory management. My C++ experience is limited, so I was hoping I could get some pointers on good memory management practices.

Here is the problem I am running into...

To describe render-able polygon, I chose to create polygon class. One of the class members is a dynamic array of vertices.

 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
class polygon
{
public:
	polygon(int vertCount)
	{
		numOfVerticies = vertCount;
		vertices = new v2[vertCount];
	}
	~polygon()
	{
		delete[] vertices;
	}

	bool isVisible;
	int numOfVerticies;
	
	v2 position;
	v2 *vertices;
};

You can see that I have a destructor that I intend for memory cleanup when the object is "done".

To init an object of class polygon, I use the following code
 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
16
17
18
19
20
typedef struct game_state
{
	polygon *player;

} game_state;

void InitPlayer(game_state *state)
{
	state->player = new polygon(4);
	polygon *p = state->player;

	p->isVisible = true;

	p->vertices[0] = v2(0, 0);
	p->vertices[1] = v2(15, 50);
	p->vertices[2] = v2(30, 0);
	p->vertices[3] = v2(15, 15);

	p->position = v2(10, 10);
}


As you can see, I use operator new to allocate memory, which means I would need to perform memory cleanup with delete. My hope was that when I delete player object, it would invoke destructor, and delete the memory allocated forverticies member.

This is what I call at clean up stage
1
delete[] gameState->player;

This does invoke polygon destructor indeed, but when it executes the line
1
delete[] vertices;

exception happens (Exception thrown: read access violation.)

I would appreciate if someone could point out if my understanding of mem clean up is incorrect and what pattern is best to follow.

Happy Holiday, Everyone!
Edited by Den V on
ratchetfreak
#10150
you didn't follow rule of 0/3/5 which essentially means that if you make a destructor you also need to create a copy constructor and a copy assign operator (with optionally the move variants).
Mārtiņš Možeiko
#10153
Rule of three is not a problem here.

If you are allocating memory with new, you need to free it with delete. If you are allocating memory with new [], you need to free it with delete []. Simple as that.

You are allocating memory with new here:
1
state->player = new polygon(4);

But you are freeing memory with delete [] here:
1
delete[] gameState->player;

This is wrong. You need to use only delete like this:
1
delete gameState->player;

By using delete[] compiler generates code that will read length of gameState->player array and then call destructor individually on each of player element. But because gameState->player is not an array (you allocated it with new, not new[]) it will read some garbage value and will call destructor too many times on garbage memory locations - and for one location "this" pointer will be garbage, that's why reading vertices member in destructor will crash.
Edited by Mārtiņš Možeiko on
Den V
#10159
mmozeiko,

Ah, I see. Thank you for your explanation!

So in general, was my idea of memory management somewhat correct? Is it typical to put delete statements in the destructor? And then expect those destructors to be executed when delete is called on the instance(s) of the object(s)? So kinda cascading delete. Or is there a better way I should organize it?
Den V
#10160
ratchetfreak,

I dont understand why copy c-tor is needed. Never heard of 0/3/5 rule, but will look it up. Thanks.
Mārtiņš Možeiko
#10161
den_v
So in general, was my idea of memory management somewhat correct? Is it typical to put delete statements in the destructor? And then expect those destructors to be executed when delete is called on the instance(s) of the object(s)? So kinda cascading delete.
Yes, that's how it works.

Though in "proper" C++ you avoid writing manual delete or delete[] statements, but rely on wrapper classes (std::vector or other "smart" pointers) to generate implicit destructors. This way you'll never forget to put delete statements, or accidentally put wrong delete like you did above.

Something like this:
 1
 2
 3
 4
 5
 6
 7
 8
 9
10
11
12
13
14
15
class polygon
{
public:
	polygon(int vertCount)
		: numOfVertices(vertCount)
		, vertices(new v2[vertCount])
	{
	}

	bool isVisible;
	int numOfVerticies;
	
	v2 position;
	std::unique_ptr<v2[]> vertices;
};


In this case compiler will generate implicit destructor that will call automatically vertices destructor. That destructor (std::unique_ptr) will execute delete or delete[]. If you don't like std:: library, it's pretty easy to make such wrapper yourself.

Rule of three also comes from "proper" C++.

In C++ objects are often used as value types. So taking your example, if you write code like this:
1
2
3
4
5
6
7
{
    polygon p1(4);
    p1.vertices[0] = ...;
    polygon p2 = p1;
    p2.vertices[1] = ...;
    ...
}

then it will crash when closing scope will be executed. Because both polygon objects p1 and p2 share same vertices array, their destructor will try to deallocate same pointer twice.

To avoid this mistake a simple rule can be followed - if you are explicitly defining one of copy constructor or assignment operator, or destructor then all three must be defined. The idea is that because you are defining one of these special class methods that typically means you are dealing with some kind of memory allocation, or other resource management (like file handles, gpu resources, etc...) So if you are writing special code to free memory in destructor, then most likely exactly same code must be written in assignment operator (because you'll be assigning other objects contents to yourself and old contents are not needed anymore).

In my example with std::unique_ptr this rule is respected. I'm not defining new destructor, I'm relying on std::unique_ptr destructor (and other members). It actually gets better, in my case the example above with p1 and p2 will not compile at all! That may be exactly what you would want in this situation - for compiler to inform you about error situation, that p1 cannot be assigned to p2.
Edited by Mārtiņš Možeiko on
ratchetfreak
#10163
den_v
ratchetfreak,

I dont understand why copy c-tor is needed. Never heard of 0/3/5 rule, but will look it up. Thanks.


The compiler will auto-generate a copy constructor and copy assign which does the very basic per-member copy.

In your case that means that vertices pointer gets copied but will be deleted and left dangling (becoming invalid) if one of the objects gets destroyed.
Den V
#10234
Thank you mmozeiko and ratchetfreak. Lots of very useful info for me.
Vittorio Patriarca
#10666
I think the final choice of the data structures for your application depends on questions like "Is your Polygon class used somewhere else or only for the player(s)?", "Can the polygonal objects change the number of points?", "The number of points is always known at compile time?", "What can trigger the object to become invisible?" and many other.

Anyway, you set the player visible before initialising it. In a multi-threading environment, you may visualise an half-initialised object.
Reply to Thread
← Back to index