Search code examples
c++memory-managementsingletonraii

Will this Singleton Class Automatically Release Memory upon Destruction?


I have created a Singleton class and I am wondering if my destructor functon will automatically release memory for the static variable named instance.

Will the following code automatically release memory?

class SingletonClass
{
    SingletonClass() 
    {

    }

    ~SingletonClass()
    {
       delete this; // or should I say... delete instance;
    }

    public:

    static SingletonClass* instance;

    SingletonClass* getInstance()
    {
       if (instance != NULL)
          return instance;

       instance = new SingletonClass();
       return instance;
    }
};

PS: Is it possible to just make instance a regular Singleton variable as opposed to a pointer? Would it be better code practice?


Solution

  • This singleton class merrily injects Undefined Behavior in your program.

    You have a static member variable which has automatic storage. As a global variable, it will get constructed before your main() routine is entered, and destructed after your main() routine exits.

    Therefore, once your program terminates and exits the main() function, the destructor of your SingletonClass instance will be invoked, and it will try to delete this; however, the object was not allocated through a call to new, and calling delete for an object that was not allocated through new gives Undefined Behavior.

    You can safely remove the delete this instruction: global objects are destroyed automatically when your program terminates.

    EDIT:

    After the edit to your question, what used to be a static variable of type SingletonClass became a static variable of type SingletonClass*. I suggest you changing it back:

    static SingletonClass instance;
    
    SingletonClass* getInstance()
    {
        return &instance;
    }
    

    Actually, instance could (and probably should) even be a static local variable of function getInstance():

    SingletonClass* getInstance()
    {
        static SingletonClass instance;
        return &instance;
    }
    

    This way you would not even have to provide a global definition for a static class member variable.

    Alternatively, you can use smart pointers for taking care of the object's lifetime, but that's unnecessary here. Just declare the variable as a static local variable of getInstance(). In C++11, its initialization would be also guaranteed to be thread-safe.