mem leak in Singleton?

[Ethan]

Hi,

I have a class defined as a "Singleton" (Design Pattern). The codes are
attached below.
My questions are:
1. Does it has mem leak? If no, when did the destructor called? If yes, how
can I avoid it? Purify does not show it has mem leak.

// test if singleto class has mem leakage
#include <iostream>
using namespace std;

class Singleton {
public:
static Singleton* Instance();
~Singleton();
void print();
protected:
Singleton();
private:
static Singleton* _instance;
double* test;
};

Singleton* Singleton::_instance = 0;

Singleton::Singleton() {
test = new double[10];
for (int i=0; i<10; i++) test = i;
}

Singleton::~Singleton() {
cout << "Destructor called!" << endl;
if (test) delete [] test; //
*****************************
}

Singleton*
Singleton::Instance() {
if (_instance == 0) {
_instance = new Singleton;
}
return _instance;
}

void
Singleton:rint() {
for (int i=0; i<10; i++) cout << test << endl;
}

int main() {
Singleton::Instance()->print();
}


2. If I remove the line marked "//*********" in the destructor, purify still
shows no mem leak!!!! Obviously this is since "test" was not deleted.

Anybody has some answers?

Thanks,

 

[Karthik Kumar]

Hi,

I have a class defined as a "Singleton" (Design Pattern). The codes are
attached below.
My questions are:
1. Does it has mem leak? If no, when did the destructor called? If yes, how
can I avoid it? Purify does not show it has mem leak.

// test if singleto class has mem leakage
#include <iostream>
using namespace std;

class Singleton {
public:
static Singleton* Instance();
~Singleton();
void print();
protected:
Singleton();
private:
static Singleton* _instance;

Do not start your members with an underscore,
leave those nomenclatures to the implementation.

double* test;
};

Singleton* Singleton::_instance = 0;

Singleton::Singleton() {
test = new double[10];
for (int i=0; i<10; i++) test = i;
}

Singleton::~Singleton() {
cout << "Destructor called!" << endl;
if (test) delete [] test; //

Are you making the assumption that test would be set to null,
if new fails in the constructor. 'new' throws 'std::bad_alloc'
in case of failure.


Singleton::Singleton() {
try {
test = new double[10];
allocated = true;
for (int i=0; i<10; i++) test = i;
} catch (const std::bad_alloc & e) {
allocated = false;
}
}

Singleton::~Singleton() {
cout << "Destructor called!" << endl;
if (allocated) delete [] test;
}

add the member variable 'allocated' to
the class definition.

*****************************
}

Singleton*
Singleton::Instance() {
if (_instance == 0) {
_instance = new Singleton;
}

As a simple rule-of-thumb try to have your new / delete
close to each other as far as possible.

return _instance;
}

void
Singleton:rint() {
for (int i=0; i<10; i++) cout << test << endl;
}

int main() {
Singleton::Instance()->print();

You have a got a unnamed temporary pointer object,
pointing to a 'Singleton' object and you are not freeing it.
This is a memory leak.

 

[John Harrison]

Do not start your members with an underscore,
leave those nomenclatures to the implementation.

That is misleading advice, the name used is perfectly valid (as I suspect
you know).

john

 

[John Harrison]

Hi,

I have a class defined as a "Singleton" (Design Pattern). The codes are
attached below.
My questions are:
1. Does it has mem leak? If no, when did the destructor called? If yes,
how
can I avoid it? Purify does not show it has mem leak.

No the destructor is never called. Whether this is a memory leak depends on
how you define memory leak. I've never heard of an operating system that
doesn't reclaim all allocated memory when a program exits.

john

 

[Chris \( Val \)]

| >>
| >> class Singleton {
| >> public:
| >> static Singleton* Instance();
| >> ~Singleton();
| >> void print();
| >> protected:
| >> Singleton();
| >> private:
| >> static Singleton* _instance;
| >
| > Do not start your members with an underscore,
| > leave those nomenclatures to the implementation.
|
| That is misleading advice, the name used is perfectly valid (as I suspect
| you know).

I don't know how you think it's misleading ?

No where in that sentence can I see a suggestion
that their use here is *invalid*.

I happen to agree with the advice, btw - I find
those identifiers to be very ugly and confusing
to read.

Cheers.
Chris Val

 

[Karthik Kumar]

That is misleading advice, the name used is perfectly valid (as I suspect
you know).

I did not say it is invalid. That was more of a comment on the
style. Somehow when I read a piece of code, and if I see a symbol
starting with an underscore, I take it to be a predefined symbol
(either by the standard or by the implementation). But then, that
could be me ..

 

[John Harrison]

I did not say it is invalid. That was more of a comment on the
style. Somehow when I read a piece of code, and if I see a symbol
starting with an underscore, I take it to be a predefined symbol
(either by the standard or by the implementation). But then, that
could be me ..

If you are making style comments then I think you should say something like
'It's a bad idea to ...', 'Do not' is too categorical for a style comment,
it makes it sound as if you are saying something is an error.

I think that the OP usage of underscores is perfectly OK, even a good idea.
So for just about every style issue there are contrary opinions, which is
why I think you should be careful to distinguish between style issues and
language issues.

john

 

[Joe Gottman]

Hi,

I have a class defined as a "Singleton" (Design Pattern). The codes are
attached below.
My questions are:
1. Does it has mem leak? If no, when did the destructor called? If yes,
how
can I avoid it? Purify does not show it has mem leak.

// test if singleto class has mem leakage
#include <iostream>
using namespace std;

class Singleton {
public:
static Singleton* Instance();
~Singleton();
void print();
protected:
Singleton();
private:
static Singleton* _instance;
double* test;
};

Singleton* Singleton::_instance = 0;

Singleton::Singleton() {
test = new double[10];
for (int i=0; i<10; i++) test = i;
}

Singleton::~Singleton() {
cout << "Destructor called!" << endl;
if (test) delete [] test; //
*****************************
}

Singleton*
Singleton::Instance() {
if (_instance == 0) {
_instance = new Singleton;
}
return _instance;
}

void
Singleton:rint() {
for (int i=0; i<10; i++) cout << test << endl;
}

int main() {
Singleton::Instance()->print();
}


2. If I remove the line marked "//*********" in the destructor, purify
still
shows no mem leak!!!! Obviously this is since "test" was not deleted.

The destructor was never called. The memory will be reclaimed at the end
of the program, so this really isn't a memory leak. If you need the
destructor to be called (to do logging for instance), the way I always do it
is to add a new private member function
static void cleanup();

Define cleanup() as

void Singleton::cleanup()
{
delete _instance;
_instance = 0;
}

Then change your instance() function to as follows:

Singleton *Singleton::Instance() {
if (_instance == 0) {
_instance = new Singleton;
atexit(cleanup); //Guarantee the Singleton will be deleted after
main() returns.
}
return _instance;
}

This guarantees that your destructor will be called. Note that this is
unsafe if the Singleton is used after main() completes.

Joe Gottman