C++ templated singleton class - properly destroyed?

Question

I have written a templated singleton class in C++ but I am afraid that it is not properly destroyed. Can you advise me on that ?

my singleton.h

#ifndef SINGLETON_H
#define SINGLETON_H

template <typename T>
class Singleton
{
public:
    static T& Instance();

protected:
    virtual ~Singleton();
    inline explicit Singleton();

private:
    static T* _instance;
    static T* CreateInstance();
};

template<typename T>
T* Singleton<T>::_instance = 0;

#endif // SINGLETON_H

singleton.cpp

#include "singleton.h"
#include <cstdlib>

template <typename T>
Singleton<T>::Singleton()
{
    assert(Singleton::_instance == 0);
    Singleton::_instance = static_cast<T*>(this);
}

template<typename T>
 T& Singleton<T>::Instance()
{
    if (Singleton::_instance == 0)
    {
        Singleton::_instance = CreateInstance();
    }
    return *(Singleton::_instance);
}

template<typename T>
inline T* Singleton<T>::CreateInstance()
{
    return new T();
}

template<typename T>
Singleton<T>::~Singleton()
{
    if(Singleton::_instance != 0)
    {
        delete Singleton::_instance;
    }
    Singleton::_instance = 0;
}

and that's how I call it (with normal - not templated or anything - class Game ) Singleton<Game>::Instance().run();

Answer 1

A classic singleton looks like this (in C++):

class S
{
    public:
        static S& getInstance()
        {
            static S    instance; // Guaranteed to be destroyed.
                                  // Instantiated on first use.
            return instance;
        }
    private:
        S();
        // Dont forget to declare these two. You want to make sure they
        // are unaccessable otherwise you may accidently get copies of
        // your singleton appearing.
        S(S const&);              // Don't Implement
        void operator=(S const&); // Don't implement
};

It is simple enough that template-ing it is redundant.
Also making it a template start to have other issues that require you to have a very good linker. SO it is worth just doing it explicitly for each object you want to make a singleton.

It is worth noting that it is probably a mistake to use a singleton. The use cases were they actually work well are very limited. Best to create the object in main and pass it as a parameter to everything that needs it.

This article covers a lot of info.

Looking at your code the thing that sticks out is:

template<typename T>
Singleton<T>::~Singleton()
{
    if(Singleton::_instance != 0)
    {
        delete Singleton::_instance;
    }
    Singleton::_instance = 0;
}

You are already in the destructor of the only object. Thus calling delete on _instance is calling delete on yourself (and that must already have been done otherwise you would not be in the destructor).

As you have deduced the problem is that the object is not automatically destroyed. There are several ways of solving this in your code. The best would be to make _instance a std::auto_ptr (std::unique_ptr in C++11). This will mean as a static storage duration object that it will be destroyed at the end of the program (which will call the destructor for you).

That aside C++ code where you have pointers strewn about the place is not real C++ code. You may be using the C++ syntax but you are writing C. There is a very different style attached to C++. You should barely ever see pointers. This is because there is no ownership semantics associated with pointers and one of the big things in C++ is expressing ownership of dynamic storage duration objects.

About the only place you see pointers are in the implementation of containers and smart pointers. As a user you should be using smart pointers or containers (depending on situation) instead of pointers. Where you passing be reference using a pointer (aka C) you should use references (aka C++).

Answer 2

You should not delete singleton object in destructor as it may possible another object is referring to it once first object scope ends.

Add another static function to destruct the object. It need to call explicitly to destruct the object. Also, Add reference counting to decide whether Singleton object need to be deleted.