Any memory leak for this Singleton Implementation

[tomgee]

I saw it many times,

// T.h:

class T
{
public:
static T* instance();
private:
T() {}
~T() {}
static T* smInstance;
};

// T.cpp:

T* T::smInstance = NULL;

T* T::instance()
{
if (smInstance == NULL)
smInstance = new T();

return smInstance;
}

But I suspected there is a memory leak, because there is a free() call
for the memory allocated by new().

However, I saw it many times that I tend to doubt myself - a similiar
implementation even appears on the "c++ cook book", a very new book by
O'really.

Anybody can help to clarify? Thanks.

 

[mlimber]

I saw it many times,

// T.h:

class T
{
public:
static T* instance();
private:
T() {}
~T() {}
static T* smInstance;
};

// T.cpp:

T* T::smInstance = NULL;

T* T::instance()
{
if (smInstance == NULL)
smInstance = new T();

return smInstance;
}

But I suspected there is a memory leak, because there is a free() call
for the memory allocated by new().

However, I saw it many times that I tend to doubt myself - a similiar
implementation even appears on the "c++ cook book", a very new book by
O'really.

Anybody can help to clarify? Thanks.

I think you mean something more like:

template<class T>
class Singleton
{
// ...
};

It's not a memory leak if your program never stops running or if the OS
cleans up after you, one of which is probably true on all modern
systems. See chapter 6 of _Modern C++ Design_ for more than you ever
wanted to know about implementing singletons in C++.

Cheers! --M

 

[tomgee]

No, it's not templated. I am sorry to have used this misleading class
name.

How can you make sure the OS will clean it up, without calling a
free()? Can you please explain a little implementation here, I am only
concerned about the dynamic memory allocation and claiming?

Thanks.

 

[Ben Pope]

I saw it many times,

// T.h:

class T
{
public:
static T* instance();
private:
T() {}
~T() {}
static T* smInstance;
};

// T.cpp:

T* T::smInstance = NULL;

T* T::instance()
{
if (smInstance == NULL)
smInstance = new T();

return smInstance;
}

But I suspected there is a memory leak, because there is a free() call
for the memory allocated by new().

I suspect you meant there is *NO* call to free.

What you *should* have said is that there is no call to delete.

However, I saw it many times that I tend to doubt myself - a similiar
implementation even appears on the "c++ cook book", a very new book by
O'really.

Anybody can help to clarify? Thanks.

When is smInstance deconstructed?

Ben Pope

 

[Victor Bazarov]

I saw it many times,

// T.h:

class T
{
public:
static T* instance();
private:
T() {}
~T() {}
static T* smInstance;
};

// T.cpp:

T* T::smInstance = NULL;

T* T::instance()
{
if (smInstance == NULL)
smInstance = new T();

return smInstance;
}

Seems like this 'T' is created once and deleted never.

But I suspected there is a memory leak, because there is a free() call
for the memory allocated by new().

Huh? Care to rephrase that?

However, I saw it many times that I tend to doubt myself - a similiar
implementation even appears on the "c++ cook book", a very new book by
O'really.

O'Reilly. Unless you intended a pun (word play on "Oh, really?")

On many modern platforms, when the program ends, all of the memory it
occupied is released. If something is dynamically created and never
destroyed explicitly, it is _quite_possible_ that the memory is still
made available to the system by the system after the program ends. But
you need to consider a couple of other points. First, what if memory
is not released? What if somebody writes a memory management engine
that keeps memory around forever until the user explicitly deletes it?
In that case the rule "always 'delete' what you get from 'new'" has all
the merit it can ever have. Second, what if somebody fixes this code
in such a way that the destructor of the singleton now has side effects?
If you don't explicitly 'delete' the pointer, the side effects will never
take place. And so on...

V

 

[TB]

tomgee sade:

No, it's not templated. I am sorry to have used this misleading class
name.

How can you make sure the OS will clean it up, without calling a
free()? Can you please explain a little implementation here, I am only
concerned about the dynamic memory allocation and claiming?

Thanks.

The OS always reclaims the memory your program allocated upon exit,
wheather you delete'd it or not.

But if you want too, you could delete it yourself when your program
exits:

void foo() {
delete T::instance();
}

#include <cstdlib>

int main() {
std::atexit(&foo);
return 0;
}

TB

 

[tomgee]

Thank you all for the above threads. I was in a hurry and made some
typos and mistakes, but you are smart to understand my question through
all that.

Since "The OS always reclaims the memory your program allocated upon
exit,
wheather you delete'd it or not. ", can I say, generally, memory leak
only happens when the application is running(it keeps asking more
memory without returning), and ends all when it terminates? Well,
Suppose the occasions Victor Bazarov mentioned won't come up.

Thanks

 

[Markus Moll]

Hi

But I suspected there is a memory leak, because there is a free() call
for the memory allocated by new().

Well, it's at least bad practice, as I would expect the destructor of the
singleton object to be called before the program exits.

Anyway, I just wanted to add that a solution would be to simply use
std::auto_ptr (making std::auto_ptr friend).

Markus

 

[Jay Nabonne]

I saw it many times,

// T.h:

class T
{
public:
static T* instance();
private:
T() {}
~T() {}
static T* smInstance;
};

// T.cpp:

T* T::smInstance = NULL;

T* T::instance()
{
if (smInstance == NULL)
smInstance = new T();

return smInstance;
}

But I suspected there is a memory leak, because there is a free() call
for the memory allocated by new().

Not only does it leak, but the destructor won't be called.

If you don't use a pointer, you can make it auto-destructing:

class T
{
public:
static T& instance();
private:
T() {}
~T() {}
};

// T.cpp:

T& T::instance()
{
// constructed when first executed. Destructed when program exits.
static T t;

return t;
}

- Jay

 

[Axter]

Seems like this 'T' is created once and deleted never.


Huh? Care to rephrase that?


O'Reilly. Unless you intended a pun (word play on "Oh, really?")

On many modern platforms, when the program ends, all of the memory it
occupied is released. If something is dynamically created and never
destroyed explicitly, it is _quite_possible_ that the memory is still
made available to the system by the system after the program ends. But
you need to consider a couple of other points. First, what if memory
is not released? What if somebody writes a memory management engine
that keeps memory around forever until the user explicitly deletes it?
In that case the rule "always 'delete' what you get from 'new'" has all
the merit it can ever have. Second, what if somebody fixes this code
in such a way that the destructor of the singleton now has side effects?
If you don't explicitly 'delete' the pointer, the side effects will never
take place. And so on...

Having a singleton get explicitly deleted can cause more harm then
good, if it's called from a destructor of multiple global objects.

If you try to delete it in main, then your global objects will be
calling a deleted object, and at best, it will result in undefined
behavior, and at worse crash the program.

So it's not ALWAYS preferable to have a singleton get explicitly
deleted, and that's why you'll commonly find singleton implementation
that never calls the destructor.

 

[Axter]

Not only does it leak, but the destructor won't be called.

If you don't use a pointer, you can make it auto-destructing:

class T
{
public:
static T& instance();
private:
T() {}
~T() {}
};

// T.cpp:

T& T::instance()
{
// constructed when first executed. Destructed when program exits.
static T t;

return t;
}

If you use the above method, and the singleton gets called by a global
object's destructor that's in a different translation unit, it will
result in undefined behavior, because there's no garantee of the order
of destruction for global objects in multiple translation units, IAW
C++ standards.

If the singleton is not called from any global object's destructor,
then the above implementation would be the safer and cleaner method.

 

[JustBoo]

If you use the above method, and the singleton gets called by a global
object's destructor that's in a different translation unit, it will
result in undefined behavior, because there's no garantee of the order
of destruction for global objects in multiple translation units, IAW
C++ standards.

If the singleton is not called from any global object's destructor,
then the above implementation would be the safer and cleaner method.

Thank you. That is the first rational explanation that I can
*remember* to counter all the hand-wringing that goes with
Singletons. Mind you, I have felt queasy at times relying on an
unreliable OS to clean up myself. "But we have deadlines to
meet dammit." It's always a balance.

I've used them for years with no trouble except the angst from other
engineers. I've used them to help get younger engineers design's
up and running quickly.

I'll 'nick' the above if you don't mind.