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 the main code calls Singleton::getInstance()->someMethod() for the first time, is the instance an instance twice? Will there be a memory leak?

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

Answer 1

When the main code calls Singleton::getInstance()->someMethod() for the first time, is the instance an instance twice?

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

However, you have one problem: you were unable to create a getInstance static member function. I assume this is a typo / surveillance, because otherwise your program will not even compile. In fact, the declaration is poorly formed even as a non-static member. In addition, the constructor must be private in order to apply the notion that only getInstance can create an instance of a type.

Will there be a memory leak?

Yes, and that is what Leak Detector reports. However, this is minimal: the problem is that nothing needs to be delete for the singleton instance before your program shuts down.

Honestly, I would not worry about that. This may be one of those rare cases where a leak is acceptable, mainly because more than a "leak", it is just a one-time refusal to allocate when the process ends.

However, if you completely get rid of the pointer, you can simultaneously avoid both problems, since one of the last actions of your program 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 )

Not even a pointer needed!

This has the added benefit that the whole function becomes inherently thread safe, at least with C ++ 11.

Answer 2

new Singleton() does not have a delete correspondence, so yes, you are a resource leak.

You will return the memory when the program closes, but not all resources will be returned when the program ends.

You can fix this by making an instance of std::auto_ptr or std::unique_ptr . Or just don't use a pointer.

Answer 3

@LightnessRacesInOrbit the answer is correct and to the point. Since your question is somehow dedicated to best practices when implementing Singleton (which you should try to avoid btw), I will give you my solution. You can use something called the Curiously Recurring Template Pattern to create a Singleton base class that takes the class you want to create a Singleton as a template parameter. Once you have done it right, creating Singletons is like taking a walk in a park. Just get Foo out of Singleton<Foo> and make Foo::Foo() and Foo::~Foo() private. Here is the code I use, with comments, live on Coliru :

 // Singleton pattern via CRTP (curiously recurring template pattern) // thread safe in C++11 and later #include <iostream> #include <type_traits> // generic Singleton via CRTP template <typename T> class Singleton { protected: Singleton(const Singleton&) = delete; // to prevent CASE 3 Singleton& operator=(const Singleton&) = delete; // to prevent CASE 4 Singleton() noexcept = default; // to allow creation of Singleton<Foo> // by the derived class Foo, since otherwise the (deleted) // copy constructor prevents the compiler from generating // a default constructor; // declared protected to prevent CASE 5 public: static T& get_instance() noexcept(std::is_nothrow_constructible<T>::value) { static T instance; return instance; } // thread local instance static thread_local T& get_thread_local_instance() noexcept(std::is_nothrow_constructible<T>::value) { static T instance; return instance; } }; // specific Singleton instance // use const if you want a const instance returned // make the constructor and destructor private class Foo: public Singleton</*const*/ Foo> { // so Singleton<Foo> can access the constructor and destructor of Foo friend class Singleton<Foo>; Foo() // to prevent CASE 1 { std::cout << "Foo::Foo() private constructor" << std::endl; } // OK to be private, since Singleton<Foo> is a friend and can invoke it ~Foo() // to prevent CASE 2 { std::cout << "Foo::~Foo() private destructor" << std::endl; } public: void say_hello() { std::cout << "\t Hello from Singleton" << std::endl; } }; int main() { Foo& sFoo = Foo::get_instance(); sFoo.say_hello(); Foo& sAnotherFoo = Foo::get_instance(); // OK, get the same instance sAnotherFoo.say_hello(); Foo& sYetAnotherFoo = sFoo; // still OK, get the same instance sYetAnotherFoo.say_hello(); thread_local Foo& stlFoo = Foo::get_thread_local_instance(); // thread local instance stlFoo.say_hello(); // CASE 1: error: 'Foo::Foo()' is private // Foo foo; // Foo* psFoo = new Foo; // CASE 2: error: 'Foo::~Foo()' is private Foo* psFoo = &Foo::get_instance(); // can get pointer psFoo->say_hello(); // delete psFoo; // cannot delete it // CASE 3: error: use of deleted function 'Foo::Foo(const Foo&)' // Foo foo(sFoo); // CASE 4: error: use of deleted function 'Foo& Foo::operator=(const Foo&)' // sFoo = sAnotherFoo; // CASE 5: error: 'Singleton<T>::Singleton() [with T = Foo]' is protected // Singleton<Foo> direct_sFoo; }