Memory leak in this situation? Using new keyword over and over

Question

I'm very cautious about memory leaks, so I thought I'd have this verified. In the following example would there be a memory leak? My gut instinct says yes.

class Handler        // Class definition
{public:
  ~Handler();
  int* ptrToInts;    
};

Handler::~Handler()  // Class destructor
{
  delete[] ptrToInts; 
}

Handler handler;     // Global object


void aFunction()
{
    handler.ptrToInts = new int[20];
}


int main()
{
  bool quit = false;

  while(!quit)
    {
      aFunction(); 
    }

  return 0;
}

Would ptrToInts be creating 20 separate new ints in separate memory on the heapeach time?

Also, another question is if it weren't for the destructor, would the dynamically allocated memory be freed? Seeing as though the lifetime of the class is the duration of the program, would it do cleanup of all the "new" memory?

Edit: Thanks for the answers. The reason I'm asking this is because I'm trying to get around calling new and delete every time WndProc is called for Raw Input basically, which is how the MSDN tells you to do it. Seems very inefficient.

If you do `new` more than once and `delete[]` just once then yes, you have a memory leak. They need to be balanced. — Joachim Isaksson, Apr 29 '16 at 19:18
Use tools (like http://valgrind.org/) that will tell you if (and where) your program has memory leaks. — simpel01, Apr 29 '16 at 19:20

score 7 · Accepted Answer · answered Apr 29 '16 at 19:17

7

As soon as you re-assign your pointer without using delete[] to de-allocate that allocated memory on the heap, you create a memory leak. This will happen if you loop your aFunction() as it re-assigns the pointer every time it is called.

As for your second question, your destructor will only delete[] the last array assigned to the pointer.

answered Apr 29 '16 at 19:17

Stephen

1,237
13
21

What would happen I ran this code? I dare not run it on my computer but I tried on an online one and it stopped after throwing a bad_alloc – Zebrafish Apr 29 '16 at 19:24
@TitoneMaurice: That's exactly what will happen on your own computer. It's perfectly harmless. Your program will crash, and then your OS will reclaim the memory. – Benjamin Lindley Apr 29 '16 at 19:28
@TitoneMaurice, The System will run out of memory for your process within fractions of seconds.... And this is a very good thing that you failed fast! You don't want late failures in Software. – WhiZTiM Apr 29 '16 at 19:29
@Benjamin Lindley Sorry, the OS will reclaim the memory? I thought it will leak and won't be freed until next system bootup. – Zebrafish Apr 29 '16 at 19:31
@TitoneMaurice [See this link](http://stackoverflow.com/questions/2975831/is-leaked-memory-freed-up-when-the-program-exits) – PaulMcKenzie Apr 29 '16 at 19:35
@TitoneMaurice: No. That would be a pretty terrible OS if it allowed something like that to happen. Many low level programmers in fact purposefully allow memory leaks for objects which don't need to be cleaned up until the end of the program, precisely because it's about to be cleaned up anyway, and they don't want to waste cycles running the deallocator. – Benjamin Lindley Apr 29 '16 at 19:36
@BenjaminLindley I wouldn't call it _perfectly_ harmless. If OP has large amount of virtual memory on slow HDD, it is possible that he will hang his PC. – Revolver_Ocelot Apr 29 '16 at 19:49
@Stephen Are you sure the destructor will be called? Someone else told me it wouldn't, and I put a break point in the destructor and ran in debug and sure enough it wasn't called. – Zebrafish Apr 29 '16 at 19:49
@Revolver_Ocelot Well from the sounds of it the memory leaks by not calling delete for every new seems less disastrous than a lot of people make it out to be. Even saying never use raw pointer, use smart pointers and stuff like that. I was very surprised to hear it's not as bad as I've heard. – Zebrafish Apr 29 '16 at 19:51
@BenjaminLindley, is this why I need to reboot my Windows PC every now and than? – SergeyA Apr 29 '16 at 20:00

score 2 · Answer 2 · answered Apr 29 '16 at 19:16

2

Only delete[] frees the memory that was allocated by new. And every time you use new, you need a delete.

For the other question, based on the Documentation:

MyClass * p1 = new MyClass[5]; // allocates and constructs five objects

answered Apr 29 '16 at 19:16

Khalil Khalaf

8,448
7
47
92

score 2 · Answer 3 · edited May 23 '17 at 10:28

2

Yes, there is a memory leak when you call the function more than once, without explicitly deallocating handler.ptrToInts after each call;

void aFunction()
{
    handler.ptrToInts = new int[20];
}

//-----somewhere we see the caller

while(!quit)
    {
      aFunction(); 
    }

However, this is a trivial case of detecting leaks... You should learn to use Leak detectors and static analyzers.

See How do you detect/avoid Memory leaks in your (Unmanaged) code?

edited May 23 '17 at 10:28

Community

1
1

answered Apr 29 '16 at 19:23

WhiZTiM

19,970
3
36
56

Thanks, someone on here just pointed me to valgrind.org for leak detection. – Zebrafish Apr 29 '16 at 19:27
valgrind will not work in your case, because you have the endless-loop in your `main()`. – cwschmidt Apr 29 '16 at 19:42
@cwschmidt, a static analyzer will work. - detect the endless loop. :-) ... Just to add, Where both fails, a debugger should pass :-) – WhiZTiM Apr 29 '16 at 19:48
@WhiZTiM Sure, but the suggestion was just to use valgrind. **Valgrind itself is not aware of the endless-loop and will run until 'insufficent memory'**. A good programmer will detect the endless-loop in this example even without a static code analyzer. – cwschmidt Apr 29 '16 at 19:52

cwschmidt · Answer 4 · 2016-04-29T19:39:48.127

Of course there is a memory leak. You allocate the ints in

void aFunction()
{
    handler.ptrToInts = new int[20];
}

without deallocating the old ints first, like

void aFunction()
{
    delete [] handler.ptrToInts;
    handler.ptrToInts = new int[20];
}

would do.

Calling aFunction() will lead to "infinite" memory allocations. And your destructor, which only frees the last allocated ints, even is never called.

Why is your handler not managing it's own memory?

It's very bad practice to allocate memory outside of your object and free it inside or vice versa.

Why not implement the Handler class this way:

class Handler
{
public:
  Handler();
  ~Handler();
  void aMethod();
private:
  int* ptrToInts;    
};

Handler::Handler() {
  handler.ptrToInts = new int[20];
}

Handler::~Handler() {
  delete[] ptrToInts; 
}

void Handler::aMethod() {
  delete[] ptrToInts; 
  handler.ptrToInts = new int[20];
}

int main() {
  bool quit = false;
  Handler handler;

  while(!quit) {
    handler.aMethod(); 
  }
}

Memory leak in this situation? Using new keyword over and over

4 Answers4