neeru
neeru

Reputation: 259

singleton using template and calling destructor

I've a singleton class implemented as follows.

#include <iostream>
using namespace std;

template<class T>
class singleton{
protected:
    static T* s_instance;
public:
    T* instance(){
        if(s_instance){
            return s_instance ;
        }else{
           s_instance = new T;
           return s_instance;
        }
    }
};
template <class T>
T* singleton<T>::s_instance;

class A:public singleton<A>{
    friend class singleton;
public:
  void print_add(){
    cout<<"I AM A"<<endl;
    cout<<s_instance<<endl;
  }
  ~A(){
      //delete s_instance;
      cout<<"Dest A"<<endl;
   }
private:
    A(){}
};

class B:public singleton<B>{
    friend class singleton;
public:
  void print_add(){
    cout<<"I AM B"<<endl;
    cout<<s_instance<<endl;
 }
 ~B(){
       cout<<"Dest B"<<endl;
       //delete s_instance;
  }
private:
    B(){}
};

int main(){
    A* a, *c;
    B* b;
    a->instance()->print_add();
    b->instance()->print_add();
    c->instance()->print_add();     
}

How to call the destruct-or for this. It looks like without "delete" lines above, valgrind shows 0 memory leaks. without deleting the pointer, am I leaking the memory? or the method of implementing singleton is wrong? And for these two classes we've a common static member. Basically how the static member is different for different object here?

Thanks

Upvotes: 1

Views: 312

Answers (2)

Artem Tokmakov
Artem Tokmakov

Reputation: 1215

For every T, there's its very own singleton class specialization, which has its very own static data member. So for A and B these are different.

You do leak memory. And there are a couple of ways to solve it. If you want to keep your lazy initialization, use std::unique_ptr for s_instance. Then objects will be destroyed properly. Note that your initialization is not thread-safe.

You can also have just: T s_instance instead of T* s_instance. This way object is constructed before main() and will also be properly destructed. This also means this is thread safe.

Yet another way is having static T s_instance; right inside instance() method and return it. This is guaranteed to be thread safe in C++11.

Upvotes: 2

Chris Hayden
Chris Hayden

Reputation: 1144

There are a few things worth noting:

Practically speaking, you are not leaking memory. Only a single instance of the class can be created (meaning there is no way the leak can lead to excessive resource use) and the memory allocated to that instance will be reaped by the OS when the client process terminates.

The simplest way to make sure the singleton instance is deleted during program termination, as opposed to reaped by the OS, is just to use a static instance with function scope:

template<class T>
struct Singleton {
  static T& instance() {
    static T instance_;
    return instance_;
  }
};

class SingletonClient : public Singleton<SingletonClient> {
  friend class Singleton<SingletonClient>;

  SingletonClient()
  {}
};

SingletonClient &s = Singleton<SingletonClient>::instance();

There is some subtlety to implementing singletons with templates. If you use a singleton template instantiation in multiple translation units then you might actually end up with more than one instance of the singleton client when you really only wanted one. The way to deal with this is to use an extern template declaration in the header file of the client class and a template instantiation in the implementation file of the client.

// In header file of SingletonClient:
extern template class Singleton<SingletonClient>;

// In the implementation file of SingletonClient:
template class Singleton<SingletonClient>;

Upvotes: 2

Related Questions