CODEWITHSUNDEEP
c++memory-management
mmirzadeh
mmirzadeh

Reputation: 7089

Is this considered a memory leak?

Suppose you have a simple class like this:

class foo{
private:
    int* mData;
    int  mSize;
public:
    foo(int size){
        mSize = size;
        mData = new int [mSize];
    }
    ~foo() {
        mSize = 0;
        delete [] mData;
    }
};

Then inside main you do:

int main () {
    static int HUGE = 100000000;
    foo a(HUGE);
    // do something useful with a
    // .
    // .
    // .
    // Now I'm done with a; I do not need it anymore ...
    foo b(HUGE);
    // do something useful with b
    // Ok we are done with b
    return 0;
}

As you can see a is no longer needed after b, but since it is created on the stack, the destructor won't be called up until the end of the program. Now, I know this is not the same as allocating with new and forgetting to call delete, however this is still wasting memory. Do you consider this as "memory leak" or just a bad programming?

Also, How would you avoid situations like this? One way would be to manually call the destructor when the object is not needed anymore, but, besides looking ugly and unfamiliar!, you get into trouble of double free unless you change the destructor to something like:

foo::~foo(){
    if (mData != NULL){
        delete [] mData;
        mData = NULL;
        mSize = 0;
    }
}

Another way is to create a on the heap via foo *pa = new foo (HUGE) and then call delete pa once the object is no longer needed. This works but at the danger of introducing another possible memory leak (if one forgets to call delete pa).

Is there any better way to get rid of unneeded objects?

Upvotes: 3

Views: 289

Answers (10)

Open AI - Opting Out
Open AI - Opting Out

Reputation: 24164

Extract functions to reduce the scope. Give them good names:

void do_a(int amount)
{
    foo a(amount);
    // ask `a` to be useful
}

void do_b(int amount)
{
    foo b(amount);
    // ask `b` to be useful
}

int main () {
    static int HUGE = 100000000;

    do_a(HUGE);
    do_b(HUGE);

    return 0;
}

Upvotes: 0

James Kanze
James Kanze

Reputation: 154047

It's not a memory leak, but if you have a variable that you need the first half of a function, and not the second, there's a good chance that the function is doing to much, and should be refactored into two (or more) separate functions.

Upvotes: 0

Specksynder
Specksynder

Reputation: 843

The constructor of the class could also take a block of memory you allocated in your 'main()' function as a parameter. That way, once 'a' is done with the use of the memory block, you can pass it into 'b' as well. 'foo' destructor does not need to release any memory at all, and you don't need to be worried about wasting memory, or object lifetimes at all.

Upvotes: 1

SigTerm
SigTerm

Reputation: 26439

Do you consider this as "memory leak"

No, unless you do something like longjmp in the middle.

or just a bad programming?

I consider using new[] to allocate array in your class bad programming practice, because you have std::vector for that.

Also, How would you avoid situations like this?

Enclose foo into scope:

{
    foo a(HUGE);
}

unless you change the destructor to something like:

delete ignores null pointers. Destructor is called only once, so no need to zero variables. Calling destructor manually is a VERY BAD IDEA - it isn't meant for that. If you want to reinitialize the structure, implement clear() or resize() methods.

Is there any better way to get rid of unneeded objects?

Yes, enclose them into scopes.

Upvotes: 0

Douglas Leeder
Douglas Leeder

Reputation: 53285

It's not a memory leak; however it precisely the sort of memory usage that Firefox developers have spent a long time fixing.

Scope is probably the easiest way to fix this, as Dark Falcon suggests. Alternatively move the allocations and related code into separate functions.

Also pointers can be more safely handled with std::auto_ptr so that they are freed when the scope is released.

Upvotes: 0

Alessandro Teruzzi
Alessandro Teruzzi

Reputation: 3978

Do you consider this as "memory leak" or just a bad programming?

No, it is not memory leak.

How would you avoid situations like this?

Write small functions, few lines. Your code will be more readable and the unused variable allocated on the stack will be freed.

Upvotes: 0

Rafał Rawicki
Rafał Rawicki

Reputation: 22710

This is not a memory leak, because you don't loose track of your allocated memory. However this is slightly ineffective, especially when the program is running longer, and should be avoided.

You can use scopes to shorten the lifetime of an object:

int main () {
    static int HUGE = 100000000;
    {
        foo a(HUGE);
        // do something useful with a
        // .
        // .
        // .
        // Now I'm done with a; I do not need it anymore ...
    }
    {
        foo b(HUGE);
        // do something useful with b
        // Ok we are done with b
    }
    return 0;
}

Also, it is worth reconsidering if this two parts of code should be in separate functions, then the allocated objects will be freed when returning from function.

Upvotes: 1

Michael Dorgan
Michael Dorgan

Reputation: 12515

Just place your huge a and b objects into their own braces if you are worried about scope.

And this isn't technically a memory leak, but it is very poor memory management as you have stated. 


{
  {
    foo a(HUGE);
  }
  ...

  {
    foo b(HUGE);
  }

Upvotes: 3

Luchian Grigore
Luchian Grigore

Reputation: 258698

No, it's definetely not a memory leak.

A memory leak is when you allocate memory and you lose its handle, so you can't free it afterwards. It doesn't matter where or when you free the memory, as long as you do.

You could add an enclosing scope to force memory freeing:

{
    foo a(HUGE);
}
{
    foo b(HUGE);
}

Upvotes: 1

Dark Falcon
Dark Falcon

Reputation: 44201

Destructors are called when an object goes out of scope. C++ allows arbitrary scopes inside function bodies. Write your main function this way:

int main () {
    static int HUGE = 100000000;

    {
        foo a(HUGE);
        // do something useful with a
        // Now I'm done with a; I do not need it anymore ...
    }

    {
        foo b(HUGE);
        // do something useful with b
        // Ok we are done with b
    }
    // etc.
    return 0;
}

I see your example is simplified, but in a real program, don't forget to either

  • implement an appropriate copy constructor and operator= for foo or
  • add a declaration for a private copy constructor and operator= so it cannot be called.

Upvotes: 10

Related Questions