Memory leak in singleton class in c++

Question

i have a singleton class. I am creating an object of it. Using it. Once it will go out of scope destructor will be called.

Again creating anew object, since instanceFlag is false, it will again allocate new memory.

#include <iostream>

using namespace std;

class Singleton
{
private:
    static bool instanceFlag;
    static Singleton *single;
    Singleton()
    {
        //private constructor
    }
public:
    static Singleton* getInstance()
    {
        if(! instanceFlag)
        {
            single = new Singleton();
            instanceFlag = true;
            return single;
        }
        else
        {
            return single;
        }
    }
    void method()
    {
        cout << "Method of the singleton class" << endl;
    }
    ~Singleton()
    {
        instanceFlag = false;
    }
};

bool Singleton::instanceFlag = false;
Singleton* Singleton::single = NULL;



int main()
{
    Singleton *sc1,*sc2;
    {
        sc1 = Singleton::getInstance();
        sc1->method();
        delete sc1;
    }
    sc2 = Singleton::getInstance();
    sc2->method();

    return 0;
}

My doubt is what will happen to old memory? I think its a memory leak. If yes how to fix this issue in the given code?

Any comments will be helpful to understand the internals.

Question: You are taking a pointer to it, so how does it go out of scope? The pointer can go out of scope, but the object pointed to cannot. — kfsone, Oct 27 '13 at 05:40
possible duplicate of [C++ Singleton design pattern](http://stackoverflow.com/questions/1008019/c-singleton-design-pattern) — Basilevs, Oct 27 '13 at 05:41
when we allocate memory dynamically, but some how lose the way to reach that memory, then it is called memory leak. And this is happening in the above case. — Devesh Agrawal, Oct 27 '13 at 05:44
if we have delete then how this code will react? int main() { Singleton *sc1,*sc2; sc2 = Singleton::getInstance(); { sc1 = Singleton::getInstance(); sc1->method(); delete sc1; } sc2->method(); return 0; } — Devesh Agrawal, Oct 27 '13 at 05:52
^ I think there should be `delete` in the destructor insteade of in `main` — Sam, Oct 27 '13 at 05:55

kfsone · Answer 1 · 2013-10-27T20:40:47.943

2

Pointers are variables that contain an address of something in memory. Like any other variable, a pointer can go out of scope, but that has no bearing on the information it is pointing to. This is how leaks happen.

{
    char* p = new char[128];
}

When p goes out of scope, the address of the allocation goes away, but the allocation is untouched; it is still allocated and its contents unaffected any more than throwing away an envelope affects the house it was addressed to.

To solve that you would need something with ref counts, like std::shared_ptr/std::weak_ptr or a RAII container like std::unique_ptr, etc. Something that has a destructor to go out of scope with.

It's better to implement the Singleton pattern with references.

class Singleton {
    Singleton() {
        // constructor code...
    }
    static Singleton s_singleton;
public:
    static Singleton& GetSingleton() {
        return s_singleton;
    }
};

or

class Singleton {
    Singleton() = delete; // if you don't have a ctor, lets not have one at all.
public:
    static Singleton& GetSingleton() {
        static singleton;
        return singleton;
    }
};

edited Oct 27 '13 at 20:40

answered Oct 27 '13 at 05:44

kfsone

23,617
2
42
74

I like this answer. I must say though, I prefer using `= delete;` instead of private constructor. – Brandon Oct 27 '13 at 05:55
1

Oh - it's actually implemented, not deleted - just an empty body because I didn't have anything to put in it. – kfsone Oct 27 '13 at 05:59
There - I did one variant in each example :) – kfsone Oct 27 '13 at 06:00
unique_ptr is not ref counted. You're thinking of shared_ptr/weak_ptr. – SoapBox Oct 27 '13 at 06:03
I like this answer because it is clear, to the point, and illustrates the problem in simple, clear to understand terms with code. – ExpatEgghead Oct 27 '13 at 06:47

score 0 · Answer 2 · answered Oct 27 '13 at 05:38

0

I use valgrind which is easy to use, it is used to detect memory leak. http://valgrind.org/docs/manual/quick-start.html

See headfirst c tutorial to detect memory leak.

answered Oct 27 '13 at 05:38

Madan Ram

880
1
11
18

score 0 · Answer 3 · answered Oct 27 '13 at 05:43

sc1 and sc2 are pointers, they are not Singleton objects. There is no call to Singleton::~Singleton() when they go out of scope.

Your problem lies elsewhere. When you delete sc1, you're not setting Singleton::instanceFlag to false. That means that sc2->method() dereferences a pointer to a deleted object.

You should make the destructor private instead and introduce a new member function for deleting the object. Also, you don't really need Singleton::instanceFlag. Simply check for Singleton::single == NULL. When deleting it, set it to NULL again.

score 0 · Answer 4 · answered Oct 08 '20 at 08:16

First of all, the way you have implemented singleton pattern is wrong,

if(! instanceFlag)
{
    single = new Singleton();
    instanceFlag = true;
    return single;
}
else
{
    return single;
}

the whole point of singleton pattern is not have multiple instance of the class, but just one. using a flag like above is not a good implementation, you are not setting flag to true and hence making new allocation of the singleton class, which indeed violated the rule of singleton pattern

You can use something like below

===========================================================

public:
    static Singleton* getInstance()
    {
        if(! single )//check if class is already instantiated once (allocation already 
          //done)
        {
            single = new Singleton();
            instanceFlag = true;
        }
            return single;
    }

OR you can also use non pointer

public:
    static Singleton& getInstance()
    {
        static Singleton single
        return single;
    }

score -2 · Answer 5 · edited May 23 '17 at 12:06

-2

Singletons are not supposed to be deleted.

Your question is already answered.

edited May 23 '17 at 12:06

Community

1
1

answered Oct 27 '13 at 05:41

Basilevs

22,440
15
57
102

Memory leak in singleton class in c++

5 Answers5