• Advertisement
Sign in to follow this  

Is this a memory leak?

This topic is 4798 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

I was reading a couple of books last night, and have stumbled across something I'm not sure I understand. It's to do with memory leaks (it's from item 10 in Meyer's Exceptional C++, although it's hinted at in Modern C++ Design as well). So simply : Is this a memory leak:
struct MyClass
{
 MyClass(int x) : x_(x) {}
 int x_;
};

int main()
{
 MyClass* myClass = new MyClass(1);
 return 0;
}

Now I always thought it was, because the memory is never freed with delete. But - I think what I'm reading is suggesting that it's not. The rationale for it not being a leak is that the pointer to memory is never lost, there is no accumulating memory drain, and "...all modern operating systems take care of completely deallocating a process's memory upon termination." (Modern C++ Design). Hence a memory leak is more like this:
struct MyClass
{
 MyClass(int x) : x_(x) {}
 int x_;
};

int main()
{
 MyClass* myClass;
 for(;;)
 {
  myClass = new MyClass(1);
// Some breaking clause
 }
 return 0;
}

In this case there is accumulating memory, and pointers to it are lost - hence memory leak. So - does the first case above reclaim allocated memory once the program is ended, or do both not reclaim memory? As a corollary : take your typical lazy-initialisation singleton example:
class Singleton
{
private:
 static Singleton* instance_;
// Ctor, copy ctor, etc.
public:
 static Singleton* Instance();
 void Kill();
};

Singleton* Singleton::instance_ = 0;

Singleton* Singleton::Instance()
{
 if(!instance_)
  instance_ = new Singleton;
 return instance_;
}

void Singleton::Kill()
{
 delete instance_;
}

Is it necessary to call Kill() at the end of the program? If you don't, are you leaking memory? Yours in confusion, Jim.

Share this post


Link to post
Share on other sites
Advertisement
Jim, your right in regard to the first snippet of code,

The pointer is created once, and then will get freed as the memory can be accessed in main, hence the pointer does not get lost, you can issue a delete or get C++ to do it for you.

The second snippet certainly is a memory leak, the pointer keeps getting overridden losing the memory each time.

You shouldn't need the Kill function, a simple destructor should do

eg:

CSingleton::~CSingleton()
{
if(instance_ != 0)
{
delete instance_;
instance_ = 0;
}
}

Share this post


Link to post
Share on other sites
Guest Anonymous Poster
within the scope of your program, both of those examples are _technically_ memory leaks. you've allocated memory and not freed it. the second example is worse, because you have no way to free the memory that you've allocated even if you wanted to.

however, when you are done with that program, the operating system will free your memory for you, in both of your examples. in the second example, even if you don't have a pointer to each of the memory blocks you've allocated, the OS memory manager (implementation of malloc/new) will, and when your program exits that memory will be returned to the OS.

in both cases, the OS will get the memory back when you exit.
it is worth mentioning, though, that the destructor will not be called for anything you've allocated and not freed.

the problem with memory leaks is that in many cases, you do not exit. eg. a long running game. if you have a library or subsystem that has a memory leak, you'll eventually run out of memory (one way or another.)

for your third example, if you exit the program, the OS will reclaim the memory you've used. your destructor will not be called. but who cares, right? because you've exited the program.

in general you want to always clean up your memory, though, just because it makes it easier to deal with your code -- don't take advantage of the fact that you happen to only do something in one place, when in the end, you might do it once per level, etc, and have to change the code to find where you _should have_ shutdown some system.

i'm pretty sure there are ways to allocate memory or other operating system resources which will not cleanup properly at program exit -- but anything allocated through standard new/malloc will be returned to the OS when you exit, even if you don't have a pointer to it.


Share this post


Link to post
Share on other sites
JimPrice - new MyClass(1); might use an OS-wide memory allocation routine for which memory is not released when the process dies (shmalloc()? GlobalAlloc()?). Not only can operator new() be overloaded, but even its default behaviour is implementation-defined. Additionally, the class constructor might also allocate OS resources (e.g. windows brushes and the like) which must be explicitely released. If the destructor is not called, that's a resource leak.

Process memory is not the only thing you have to worry about. Applications leaking resources that way are a cause of Windows sluggishness.

Share this post


Link to post
Share on other sites
Yup, destructors not being called, resource leaks - all tallies with what I'm reading.

I'm still a little confused at this though:

Quote:

new MyClass(1); might use an OS-wide memory allocation routine for which memory is not released when the process dies (shmalloc()? GlobalAlloc()?). Not only can operator new() be overloaded, but even its default behaviour is implementation-defined.


Are you saying that I'm not guaranteed to get allocated memory back if I just leave it to the OS, without explicitly calling delete?

Jim.

Share this post


Link to post
Share on other sites
Quote:
Original post by JimPrice
Are you saying that I'm not guaranteed to get allocated memory back if I just leave it to the OS, without explicitly calling delete?


Depends on the platform. C and C++ aren't quite concerned with what happens once a process has terminated. The C++ standard may have something to say on the topic, but I'm too lazy to check [razz] (look here for a later edit if I do bother to check [grin]). You can probably assume that, unless you've been told otherwise the memory will be reclaimed but the mere fact that operator new and operator delete can be overloaded, combined with the existence of memory allocation routines which break the process boundaries should make careful. You don't know, a priori, how the default new and delete have been implemented.

Share this post


Link to post
Share on other sites
Nope, you're not guaranteed that the OS will recover the memory when the app terminates. Most (all?) modern OS'es do it anyway, but there's no guarantee of it in the C++ standard.

Windows 98 had some problems with this. If you ran a program that leaked lots of memory, you usually had to reboot afterwards to get decent performance again. I don't know if it attempted to recover the memory, but "forgot" about some of it, or if it just didn't even try, and relied on the programmers to remember to delete their objects before their app terminated.

Share this post


Link to post
Share on other sites
well what exactly is unfreed memory? i would think it's simply the os saying it's not free so if the os doesn't "have pointers to used memory" it simply is by definition free no?
once the os done freeing all the memory it allocated for a program i don't see how it could still have unfreed memory (since if it's the OS itself that doesn't know some memory was allocated it is by definition free since it won't refuse it to other programs).or am i missing something obvious?

Share this post


Link to post
Share on other sites
What you are doing may very well be cleaned up by the OS when the program exits, however it is definately not Kosher to leave it like that.
When it comes time to use a tool to check for real memory leaks it will be harder to distinguish the real leaks from the one-time-leaks.
It's also possible for you design to change and what was a singleton with a single leak, is now not a singleton, due to a design change. Hence by not fixing it earlier, the leak multiplies.

The moral of the story is: Always be nice to your memory manager and give back what you borrow before you die.

Share this post


Link to post
Share on other sites
Quote:
Original post by ranakor
or am i missing something obvious?


What you are missing is that there are resources, memory and otherwise, that do exist outside of your process memory space and can indeed be shared by multiple processes. The OS cannot track what you do with the pointers (or handles) to those resources it gave you. Until and unless you tell the OS you are done with them, these entities will be kept alive by the OS, whether or not you can access them or not. It is a memory leak, but at the OS level, not at the process level.

Share this post


Link to post
Share on other sites
I would consider it fairly safe to not free() process-wide normally allocated blocks of memory - matter of fact some libraries do this by design, which may clutter up your favorite memory leak detecting tool, forcing you to write scripts and the like to filter out the "normal/okay" "leaks".

That said, I'd free them just for completeness, or at least do something like make a function like so:

inline void exiting_free( void * ptr )
{
if ( DEBUG_MEMORY_LEAKS ) free( ptr );
}


(edit: that is, that way one can stop all the "leaks" for debugging)

However, when you start using C++'s new/delete, those arn't limited to just memory. You can put ALL SORTS of stuff in destructors. Proper closing handshakes over net connections, writing application-cached data to files, releasing resources that your OS is too dumb to reclaim, releasing resources that your OS isn't supposed to automatically reclaim (temporary files in folders other than OS-labeled temp directories)...

The list goes on and on. So, write like that example of yours in any program/library/whatever I work on and I will kill you X_x.

(Kidding, I'll just be ticked ^_^;)

[Edited by - MaulingMonkey on January 6, 2005 2:38:33 PM]

Share this post


Link to post
Share on other sites
Quote:
Original post by Fruny
Quote:
Original post by ranakor
or am i missing something obvious?


What you are missing is that there are resources, memory and otherwise, that do exist outside of your process memory space and can indeed be shared by multiple processes. The OS cannot track what you do with the pointers (or handles) to those resources it gave you. Until and unless you tell the OS you are done with them, these entities will be kept alive by the OS, whether or not you can access them or not. It is a memory leak, but at the OS level, not at the process level.


sorry i was unclear what i meant was in response to OS unloading memory once a program close

since memory held is not "physically" held but simple "refused" to other processes by the os how could the os not be able to free it (aka re start accepting it when other processes ask for it) once a program holding said memory terminates

Share this post


Link to post
Share on other sites
Quote:
Original post by MaulingMonkey
I would consider it fairly safe to not free() process-wide normally allocated blocks of memory - matter of fact some libraries do this by design, which may clutter up your favorite memory leak detecting tool, forcing you to write scripts and the like to filter out the "normal/okay" "leaks".

That said, I'd free them just for completeness, or at least do something like make a function like so:

inline void exiting_free( void * ptr )
{
if ( DEBUG_MEMORY_LEAKS ) free( ptr );
}


(edit: that is, that way one can stop all the "leaks" for debugging)

However, when you start using C++'s new/delete, those arn't limited to just memory. You can put ALL SORTS of stuff in destructors. Proper closing handshakes over net connections, writing application-cached data to files, releasing resources that your OS is too dumb to reclaim, releasing resources that your OS isn't supposed to automatically reclaim (temporary files in folders other than OS-labeled temp directories)...

The list goes on and on. So, write like that example of yours in any program/library/whatever I work on and I will kill you X_x.

(Kidding, I'll just be ticked ^_^;)
I very much doubt that some libraries do this by design. I certainly hope no one would ever do that by design, but more likely by mistake, or out of sheer laziness.

For the record, I think you idea of only freeing certain memory allocations in debug mode is a REALLY BAD idea. If you have gone to the trouble of making sure that you can free it correctly in a debug build then it would be ridiculous to omit it in the release build. Correctly freeing memory is NOT merely a debugging tool, and should never be treated as such. There is no excuse to not always correctly free memory.
Just thinking what possibly important actions certain destructors might perform (now or in the future), going unexecuted could do, should make you worried enough to never not 'delete' something. 'free' should be treated the same IMHO. But hey, that's just me...
It's certainly not a practice I would recommend you show any potential employers though anyway!

Share this post


Link to post
Share on other sites
In a post in For Beginners, iMalc said:
> stuff

Me too. I concur.

Share this post


Link to post
Share on other sites
Sign in to follow this  

  • Advertisement