Singleton: is there a memory leak?

Question

This is a simple singleton:

class Singleton
{
    Singleton();
    virtual ~Singleton();

    Singleton * Singleton::getInstance() 
    {
        static Singleton * instance;

        if (!instance) {
            instance = new Singleton();
        };
        return instance;
    };
}

When main code calls Singleton::getInstance()->someMethod() for the first time, isn't the class instantiated twice? Will be there memory leak?

I am asking because Visual Leak Detector detects memory leak on the line with new Singleton().

Answer 1

When main code calls Singleton::getInstance()->someMethod() for the first time, isn't the class instantiated twice?

No. Calling a static member of Singleton does not instantiate Singleton, so the only instance of Singleton to exist here is the one you create with new. And you only create it once, because once instance points to it, you never call new again.

One problem you have here, however, is that you failed to make getInstance a static member function. I assume this is a typo/oversight since, otherwise, your program would not even compile. In fact, the declaration is ill-formed even as a non-static member. Furthermore, the constructor should be private to enforce the notion that only getInstance may instantiate the type.

Will be there memory leak?

Yes, and this is what Leak Detector is reporting. However, it's minimal: the problem is that there is nothing to delete that singleton instance right before your program shuts down.

Frankly I wouldn't worry about it. This may be one of those rare times that a leak is acceptable, mostly because more than a "leak" it's just a one-time failure to de-allocate on process shutdown.

However, if you get rid of the pointer entirely then you can avoid both problems at the same time, since one of the last things your program does will be to destroy objects of static storage duration:

#include <iostream>

class Singleton
{
public:
    ~Singleton()  { std::cout << "destruction!\n"; }

    static Singleton& getInstance() 
    {
        static Singleton instance;
        return instance;
    }

    void foo() { std::cout << "foo!\n"; }

private:
    Singleton() { std::cout << "construction!\n"; }
};

int main()
{
    Singleton::getInstance().foo();
}

// Output:
//   construction!
//   foo!
//   destruction!

(live demo)

No need even for a pointer!

This has the added benefit that the entire function becomes inherently thread-safe, at least as of C++11.