C++, destructor of a singleton class is called once again

Question

I made a simple singleton class. While running test, I got some weired result.

Destructor is called once again.

Here is result and my code.

Result : I expect destructor is called 4 times, because I called GetInstance() 4 times. But Desctuructor is called 5 times!

Start Test
TestClass Constructor
   TestClass Destructor
   TestClass Destructor
   TestClass Destructor
   TestClass Destructor
   TestClass Destructor

singleton.h

#ifndef SINGLETON_H_
#define SINGLETON_H_

#include "basictype.h"

namespace common {
namespace internal {

// Usage :
// MyClass mine = common::internal::Singleton<MyClass>::GetInstace();
// mine.foo();

// This Singleton class is maybe the best one avoiding memory allocation.
// See http://stackoverflow.com/questions/1008019/c-singleton-design-pattern/1008289#1008289
template <typename Type>
class Singleton {
 public:
  static Type& GetInstance() {
    static Type instance;
    return instance;
  }
 private:
  Singleton() {};

  DISALLOW_COPY_AND_ASSIGN(Singleton);
};

}  // namespace internal
}  // namespace common

#endif  // SINGLETON_H_

main.c

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

class TestClass {
 public:
  TestClass() {
    std::cout << "TestClass Constructor" << std::endl;
  }
  ~TestClass() {
    std::cout << "   TestClass Destructor" << std::endl;
  }
};

void runSingletonTest() {
  TestClass tc = common::internal::Singleton<TestClass>::GetInstance();
  TestClass tc2 = common::internal::Singleton<TestClass>::GetInstance();
  TestClass tc3 = common::internal::Singleton<TestClass>::GetInstance();
  TestClass tc4 = common::internal::Singleton<TestClass>::GetInstance();
}

int main(){
  std::cout << "Start Test" << std::endl;
  runSingletonTest();
  return 0;
}

That's not really a singleton, right? Your constructor&destructor should be private; the instance created by the static declaration inside GetInstance should be the only one (if you follow correctly the pattern). — Fabio Ceconello, Jun 20 '13 at 02:27

score 8 · Accepted Answer · edited May 23 '17 at 12:05

You actually have 5 instances of TestClass in your code.

The first one is created by

static Type instance;

using the default constructor. This produces the line TestClass Constructor in your output.

The other 4 are created by

TestClass tc = common::internal::Singleton<TestClass>::GetInstance();
TestClass tc2 = common::internal::Singleton<TestClass>::GetInstance();
TestClass tc3 = common::internal::Singleton<TestClass>::GetInstance();
TestClass tc4 = common::internal::Singleton<TestClass>::GetInstance();

using the copy constructor. The copy constructor is generated by the compiler and does not output anything (That's why you see only one TestClass Constructor in your output). Therefore, there are 5 instances of TestClass which get destroyed.

Note: Your Singleton class is not really a singleton. To follow the singleton pattern correctly, you should disallow copying and assigning by declaring the (copy) constructors and destructors as private:

template <typename Type>
class Singleton {
public:
  static Type& GetInstance() {
    static Type instance;
    return instance;
  }
private:
  Singleton() {}
  ~Singleton() {}

  // Dont forget to declare these two. You want to make sure they
  // are unaccessable otherwise you may accidently get copies of
  // your singleton appearing.
  Singleton(const Singleton&);                // Don't Implement
  Singleton& operator=(const Singleton&);     // Don't implement
};

There are some helpful discussions about C++ Singleton design pattern.

If C++11 is available, it's better to use

private:
  Singleton() = default;
  ~Singleton() = default;

  Singleton(const Singleton&) = delete;
  Singleton& operator=(const Singleton&) = delete;

This ensures that nobody, not even the class itself, can call the copy or copy assignment functions.

If C++11 is available it's better to replace those `; // Don't Implement` with `= delete;`. This ensures that nobody, not even the class itself, can call the copy or copy assignment functions. And you can use `= default;` instead of empty braces for the private constructor and destructor. — bames53, Jun 20 '13 at 03:43

score 1 · Answer 2 · answered Jun 20 '13 at 02:39

So having a Singleton template class is overkill and not necessary. Singleton is a design pattern, not a type.

For each class T you want to make a singleton, simply:

add the instance static function
make private the default constructor (and only call it in instance method)
delete the copy constructor

This will prevent more than one instance of type T from being created.

Try the following:

class TestClass {
public:

  // 1
  static TestClass& instance() { static TestClass x; return x; }

private:

  // 2
  TestClass() {
    std::cout << "TestClass Constructor" << std::endl;
  }

  // 3
  TestClass(const TestClass&) = delete;

  ~TestClass() {
    std::cout << "   TestClass Destructor" << std::endl;
  }
};

void runSingletonTest() {
  TestClass& tc = TestClass::instance();
  TestClass& tc2 = TestClass::instance();
  TestClass& tc3 = TestClass::instance();
  TestClass& tc4 = TestClass::instance();
}

Now you have 4 references to the same object, and you can't accidentally create a second TestClass.

score 0 · Answer 3 · answered Jun 20 '13 at 02:21

0

The "static Type instance" in your template is creating another instance.

answered Jun 20 '13 at 02:21

mike__t

979
5
6

score 0 · Answer 4 · edited Jun 20 '13 at 02:27

0

You use the Meyer´s singleton pattern with mistake.

getInstance should have form:

Singleton & GetInstance() {
   static Singleton instance;
   return instance;
}

edited Jun 20 '13 at 02:27

D.Shawley

58,213
10
98
113

answered Jun 20 '13 at 02:23

Spoonwalker Highwater

411
2
9

It will take a good bit more than this fix. There is no restriction against creating copies of `TestClass`. – D.Shawley Jun 20 '13 at 02:27

score 0 · Answer 5 · answered Jun 20 '13 at 07:16

To get it working you should:

Make Singleton's constructor, copy constructor and assignment operator protected (not private).
Derive your TestClass from Singleton template.

class TestClass : public Singleton { ... };

Try and let me know if it works for you ;)

C++, destructor of a singleton class is called once again

5 Answers5