Resource leak from an array

Question

I have following class:

class estimate
{
    public:
        estimate();
        ~estimate();
        double *tHanning;
}

estimate::estimate()
{
    tHanning = NULL;
    tHanning = new double [1000];

    for (int m=0; m<1000; m++)
    {
        tHanning[m]=(0.5-0.5*cos(2.0*PI*(m+1)/(1001))); 
    }
}

estimate::~estimate()
{
    delete [] tHanning;
    tHanning = NULL;
}

I am not sure why C++ Memory Validator shows resource leak in the constructor when I assign "new" to the variable.

Can somebody please help me?

Edit: How I sue the above class:

class HBMain
{
    public:
        HBMain();
        ~HBMain();
        bool Init();
        estimate *objEstimate;
}

HBMain :: HBMain()
{
    objEstimate = NULL;
}

HBMain :: ~HBMain()
{
    delete objEstimate;
    objEstimate = NULL;
}

bool HBMain :: Init()
{
    ....
    objEstimate = new estimate();
    ....
}

Please post the exact error message. Also, how do you create instances of `estimate`? — arne, Jul 29 '13 at 13:46
How do you use the `estimate` class? You might want to read about [the rule of three](http://en.wikipedia.org/wiki/Rule_of_three_%28C%2B%2B_programming%29). — Some programmer dude, Jul 29 '13 at 13:47
Thanks all. I have edited and put the code on how I use the estimate class. — chintan s, Jul 29 '13 at 13:56
This is why it's recommended to use `std::vector` instead of doing manual memory management. — Nikos C., Jul 29 '13 at 13:57
You may have made a mistake while adding your code: Why are you using objEstimate as it was a pointer, it's not declare as Estimate* ! — alexbuisson, Jul 29 '13 at 14:00
Sorry guys, I have edited again. I have indeed declared it as a pointer. — chintan s, Jul 29 '13 at 14:02
Is this a simplification of the code? For example, is estimate inherited without a virtual destructor? — Joachim Isaksson, Jul 29 '13 at 14:13
@JoachimIsaksson This is a simplification but it is not inherited. — chintan s, Jul 29 '13 at 14:23
Many peopel have pointed out to use vector::double? What are the advantages and disadvantages? Will it not take more memory? — chintan s, Jul 29 '13 at 14:24
@chintans `std::vector` consumes no more memory than an array of `double` of the same size. Its advantages are that you don't have to worry about manually managing memory. Its disadvantage is... well... I don't know really. Can't think of anything off the top of my head. — Nik Bougalis, Jul 29 '13 at 14:52

Rak · Answer 1 · 2013-07-29T14:10:01.767

1

As an alternative solution, why not simply change your pointer by a vector of double ?

You will avoid headaches about memory leaks.

Edit: For the HBMain class, you can also changed your naked pointer by a smart pointer (shared_ptr) from C++11 or the Boost library and delete the destructor. And so you will not have to implement all the boilerplate code.

But, do you really need a dynamic allocation for the HBMain property ?

edited Jul 29 '13 at 14:10

answered Jul 29 '13 at 14:00

Rak

76
4

Many thanks. If I use vector::double, will it not take more memory? – chintan s Jul 29 '13 at 14:25
Yes a bit more and it will also be a bit more slower. But you have to keep in mind that C++ is really fast so for the vast majority of code, that's not a big deal. In advantage : no memory leak headache and that's a huge plus. And it's the same of the smart pointers. – Rak Jul 29 '13 at 14:27
Thanks again. Let me convert all the pointers to vector and see how much memory it takes. Cheers. – chintan s Jul 29 '13 at 14:30
Also, of course you will not have to implement boilerplate code. – Rak Jul 29 '13 at 14:31
You only have to change tHanning to a vector. For objEstimate, it will be a shared_ptr. If you can not use C++11, you can still use smart pointers by using the Boost library. – Rak Jul 29 '13 at 14:33
This is just one issue. I have many more such pointers! not much but enough to leak plenty of memory. – chintan s Jul 29 '13 at 14:36

score 1 · Answer 2 · edited May 23 '17 at 12:05

Both your estimate class's constructor and destructor seem fine (you dynamically allocate memory with new[] in the constructor, and delete[] it in the destructor).

But I think you may have problems during copies.

In fact, your class has default copy constructor and operator=, but the default behavior is not good in this case; in fact, the default behavior is just member-wise copy, but copying the raw pointer tHanning data member is a "leaktrocity".

Either disable copy constructor and operator= (e.g. declaring them private in C++98/03, or using the new = delete C++11 syntax), or give a proper implementation for them.

Proper implementation should do deep copy of the owned array (with proper deletion of any previously allocated array).

I think the best thing you can do is to use std::vector<double> instead of the raw pointer.

If you are really such highly constrained on the (relatively small) overhead of std::vector, then consider using a smart pointer like std::unique_ptr:

#include <memory>   // for std::unique_ptr

class estimate
{
private:
    std::unique_ptr<double[]> tHanning; // <--- SMART pointer

public:
    estimate()
        : tHanning(new double[1000])
    {
        for (int m = 0; m < 1000; m++)
        {
            tHanning[m] = (0.5-0.5*cos(2.0*PI*(m+1)/(1001))); 
        }
    }

   // No need to define destructor - smart pointer automatically deletes its owned stuff
};

In this way, the class will be movable but not copyable, and you will incur almost zero overhead if compared to the raw pointer case.

As a bouns reading, consider the Rule of Three in C++.

Again, in your HBMain class, don't use a raw estimate *objEstimate pointer data member. Instead, use smart pointers like:

class HBMain
{
  ....
private:
    std::unique_ptr<estimate> objEstimate;

   // Again, no need to define a custom destructor for deleting objEstimate.
   // std::unique_ptr will do proper deletion *automatically*.
};

Many thanks for the reply. The use of number 1000 was just for an example. How do I modify the code in constructor, where I get the size from another calculation? Thanks again. — chintan s, Jul 30 '13 at 07:57
You're welcome. For your new question: you can use **`unique_ptr::reset()`** method, something like this: `const int count = GetProperSize(); tHanning.reset( new double[count] );`. — Mr.C64, Jul 30 '13 at 09:09
Thanks again for reply. So should I do the same as above, if want to change the size anywhere else in the code? — chintan s, Jul 30 '13 at 12:42
I did exactly as above and C++ Memory Validator still think it leaks memory :( — chintan s, Jul 30 '13 at 14:57

Resource leak from an array

2 Answers2