Not sure how to handle the destructor (Large vertex array in a class used as data source for other instances of the same class)

2
votes

This makes me nervous. It works but, did I just created a subtle memory leak?

Here is the story:

I have a base class that stores vertices in a vertex array for use in OpenGL stuff. My intent is to store vertex data only once into a master object and then create instances of it and I want instances to read vertex data from the master object while keeping their own transformation matrices.

Declaring a pointer to a vector array

I declare a pointer to a vector array as private (since I do not want it being manipulated by anyone else except siblings)

The tricky part is I am forced to use a constructor with a default value of pointer-to-TypeGeometry.

constructor declaration using default parameter

If the object is constructed by passing it a pointer to a sibling object then this new object does not use a vertex array of its own. If it's constructor is passed the nullptr then the object creates it's own vertex array.

As you can see in the images I have to use the 'new' operator whenever the object is a master, but if I add a delete command in the destructor everything breaks.

If I omit the delete command everything works, but I worry for memory leaks. Should I worry ?

define vertexARRAY (*VertexArray)

Constructor definition

HERE is the issue: if I enable this destructor it fails. destructor

Running fine with the destructor DISABLED. Notice only three objects were created but there are multiple calls to the destructor. enter image description here

Now see what happens when the destructor is ENABLED: enter image description here

3
c++pointersvectorconstructordestructor
sounds like you could use std::shared_ptr – Sander De Dycker
I just tried the shared pointer way, but now it fails instantly. It doesn't even finish constructing the first object: Is this right: I use -----> shared_ptr < vector < TypeCartesian> > VertexArray (new vector < TypeCartesian>); – Thomas An
Why are you posting screenshots of your code instead of copy-pasting it ? – Quentin
I think it would be a cleaner design to have a separate "data" class, instead of having one class that behaves differently depending on a flag. – Reto Koradi

3 Answers

2
votes

Yes, you have a memory leak because you don't delete memory allocated with new.

To fix this, you should just check if this is the master geometry in the destructor:

TypeGeometry::~TypeGeometry() {
    if (isMaster) delete VertexArray;
}
1
votes

As you can see program is crashing because of deleting pointer more than once . You have 2 options :

  1. use smart pointers like

    std::shared_ptr < std::vector < TypeCartesian> > VertexArray

  2. Check in destructor if that memory area is deleted on not :

    TypeGeometry::~TypeGeometry() { if (isMaster && VertexArray != NULL) { delete VertexArray; VertexArray = NULL; } }

0
votes

Success ! (This was an all nighter, but I finally got somewhere thank to all comments and ideas posted. Thank you, thank you, thank you ! )

I did not have to use smart-pointers in the end. The crux of the matter was that I was making function calls passing the class objects by value. Since I did not explicitly have a copy constructor the compiler was copying all the private variables verbatim (including the 'isMaster' flag).

The solution (as suggested) was two fold: A) To make my own copy constructor and make sure the isMaster flag is NOT copied over. This way the destructor is not getting fooled. B) Stop passing class objects by value. This was sloppiness of my part.

This turned out to be a trivial solution, but was too blind to see it.