C++ Memory leak caused by a pointer being set to NULL in the class destructor

Question

I have a class with the .h file as:

class dbsk2d_ishock_node : public dbsk2d_ishock_elm{
protected:
    dbsk2d_ishock_edge* _cShock;
    //Some other variables
public:
    dbsk2d_ishock_node (int newid, double stime, vgl_point_2d<double> Or); //Constructor
    virtual ~dbsk2d_ishock_node (); //Destructor
    //Some other functions
}

The .cxx file looks like:

//Constructor
dbsk2d_ishock_node::dbsk2d_ishock_node (int newid, double stime, vgl_point_2d<double> Or) : dbsk2d_ishock_elm (newid, stime)
{
    _cShock = NULL;
    //Some other variables
}

//Destructor
dbsk2d_ishock_node::~dbsk2d_ishock_node ()
{
    _cShock = NULL;
    //Some other variables
}

Now the constructor for this class sets _cShock = NULL; and the virtual destructor also sets _cShock = NULL;.

So when the destructor is called, it will just set the pointer to NULL but the actual memory being poined to won't be destroyed, right, causing a memory leak?

This class is called many times and after a certain point the program crashes due to excessive memory usage. How do I fix this?

Don't use protected data. Delete the pointer in the destructor. Or am I missing something here? — , May 25 '18 at 18:36
"*but the actual memory being pointed to won't be destroyed, right*" - that's correct. "*causing a memory leak?*" - that depends on whether the class owns the pointed-to object. Does it? — melpomene, May 25 '18 at 18:37
If it does own the pointed-to object, you shouldn't be using a raw pointer to begin with. — Baum mit Augen, May 25 '18 at 18:38
Why aren't you using a smart pointer class like std::unique_ptr? — Graham Asher, May 25 '18 at 18:43
I really..really want to ask question : "how do you protect data?" Naked pointers notwithstanding — Swift - Friday Pie, May 25 '18 at 18:48

JeJo · Answer 1 · 2018-05-25T19:28:55.647

So when the destructor is called, it will just set the pointer to NULL but the actual memory being poined to won't be destroyed, right, causing a memory leak?

YES. It will.

How do I fix this?

you can delete the pointer like

~className()
{
   delete dbsk2d_ishock_edge;
}

or Best practice is using Smart Pointers as member variables, by including <memory>. You can also refer this, as it look like you are not aware of them. For instance:

protected:
    std::shared_ptr<dbsk2d_ishock_edge>  _cShock;

Alberto Valero · Answer 2 · 2018-05-25T18:52:35.727

0

As you set the pointer to NULL in the constructor you could safely free the memory doing this

~className() { 
    if (dbsk2d_ishock_edge){
        delete dbsk2d_ishock_edge;
        dbsk2d_ishock_edge = NULL;
    }
}

edited May 25 '18 at 18:52

answered May 25 '18 at 18:45

Alberto Valero

410
4
14

2

Why are you checking for `!= NULL`? And why are you setting the pointer to `NULL` afterwards? – melpomene May 25 '18 at 18:46
1

I think you mean "destructor". – DaveG May 25 '18 at 18:46
3

you don't need check for NULL (or, rather, nullptr), NULL is delete's friend. – Swift - Friday Pie May 25 '18 at 18:46
@melpomene the pointer is set to NULL in the constructor. It could happen that the new is never being called, if you call delete on an not intialized pointer you could get a segfault. I set to NULL afterwards because if I have copied the ptr address to some other variable in this way I can know it does not exist anymore. Just extra safety – Alberto Valero May 25 '18 at 18:49
1

@AlbertoValero The pointer is set to `NULL` in the constructor. It is never uninitialized. On the other hand, if the pointer actually is uninitialized, then `if (dbsk2d_ishock_edge != NULL)` has undefined behavior. – melpomene May 25 '18 at 18:50
@Swift-FridayPie all my life checking... Thanks. Not any more! – Alberto Valero May 25 '18 at 18:51

C++ Memory leak caused by a pointer being set to NULL in the class destructor

2 Answers2