patterncppMinor
C++ templated singleton class - properly destroyed?
Viewed 0 times
templatedproperlydestroyedsingletonclass
Problem
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
singleton.cpp
and that's how I call it (with normal - not templated or anything - class
my singleton.h
#ifndef SINGLETON_H
#define SINGLETON_H
template
class Singleton
{
public:
static T& Instance();
protected:
virtual ~Singleton();
inline explicit Singleton();
private:
static T* _instance;
static T* CreateInstance();
};
template
T* Singleton::_instance = 0;
#endif // SINGLETON_Hsingleton.cpp
#include "singleton.h"
#include
template
Singleton::Singleton()
{
assert(Singleton::_instance == 0);
Singleton::_instance = static_cast(this);
}
template
T& Singleton::Instance()
{
if (Singleton::_instance == 0)
{
Singleton::_instance = CreateInstance();
}
return *(Singleton::_instance);
}
template
inline T* Singleton::CreateInstance()
{
return new T();
}
template
Singleton::~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::Instance().run();Solution
A classic singleton looks like this (in C++):
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:
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
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
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
Singleton::~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++).Code Snippets
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
};template<typename T>
Singleton<T>::~Singleton()
{
if(Singleton::_instance != 0)
{
delete Singleton::_instance;
}
Singleton::_instance = 0;
}Context
StackExchange Code Review Q#13615, answer score: 3
Revisions (0)
No revisions yet.