7

I asked a question about singleton implementation a few minutes ago, I've got very good answer from @LightnessRacesinOrbit.

But I cannot understand why in the next example if I instantiate Singleton in variable inst its destructor called twice?

#include <iostream>

class Singleton
{
public:
    ~Singleton()  { std::cout << "destruction!\n"; }

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

    void foo() { std::cout << "foo!\n"; }

private:
    Singleton() { std::cout << "construction!\n"; }
};

int main()
{
    Singleton inst = Singleton::getInstance();
    inst.foo();
}

Output:

construction!
foo!
destruction!
destruction!

Live demo

To be more correct, I understand why it is called twice. But I cannot understand how it can be called twice if after first destructor the instance of the class was destroyed? Why there is no exception?

Or it was not destroyed? Why?

Community
  • 1
  • 1
vladon
  • 8,158
  • 2
  • 47
  • 91
  • 7
    Add `Singleton(Singleton const&) { std::cout << "copy construction!\n"; }` to your example and all will be revealed. – Praetorian Jun 02 '15 at 18:25
  • 2
    You should make the class non-copiable and non-movable: `Singleton(Singleton const&) = delete; Singleton(Singleton &&) = delete; Singleton &operator=(Singleton const&) = delete;Singleton operator=(Singleton &&) = delete;` – bames53 Jun 02 '15 at 18:27
  • 4
    You have **two** instances of `Singleton` type in your program. One is `static` inside `getInstance`. Another is local inside `main`. Each object gets destroyed eventually. Hence two destructor calls. Why does it surprise you that the destructor is called twice? – AnT stands with Russia Jun 02 '15 at 18:27
  • 1
    @AnT because I am newbie. Thank you all. Now I understand. – vladon Jun 02 '15 at 18:29
  • 1
    I recommend reading this: http://jalf.dk/blog/2010/03/singletons-solving-problems-you-didnt-know-you-never-had-since-1995/ – Fred Larson Jun 02 '15 at 18:30
  • 4
    Sorry, I should have deleted the copy constructor and assignment operator. Although in a sense I'm glad I didn't because then you got to ask this :) – Lightness Races in Orbit Jun 02 '15 at 18:34
  • 2
    @LightnessRacesinOrbit And after getting comfortable writing and using Singleton's, they can proceed to [learning to avoid using them](https://stackoverflow.com/questions/137975/what-is-so-bad-about-singletons) :) – Cory Kramer Jun 02 '15 at 19:00

4 Answers4

18

This line

Singleton inst = Singleton::getInstance();

Should be

Singleton& inst = Singleton::getInstance();

Then you would only see one destructor call.

The way it is written, Singleton::getInstance() returns a reference, but then copies it to inst. So both the Singleton returned from your function and the copy are destroyed. You never saw the copy get constructed because the default constructor was not used, the copy constructor was.

In the second method, the reference is returned, then you just have inst be a reference to that Singleton instead of making a copy.

As other's have mentioned, you can make the class non-copyable and non-movable to prevent this

Singleton(Singleton const&) = delete;             // Copy construct
Singleton(Singleton&&) = delete;                  // Move construct
Singleton& operator=(Singleton const&) = delete;  // Copy assign
Singleton& operator=(Singleton &&) = delete;      // Move assign
Cory Kramer
  • 114,268
  • 16
  • 167
  • 218
8

The line

Singleton inst = Singleton::getInstance();

copies your instance with the auto-generated copy-constructor. To prevent this from happening, add

Singleton( const Singleton& ) = delete;

to your class to prevent those accidential copies. To make sure that even more obscure mistakes are caught, also add

void operator=( const Singleton& ) = delete;

as well. You don't have to explicitly delete move construction or assignment as the compiler will not generate them with the other (deleted) members declared.

Daniel Frey
  • 55,810
  • 13
  • 122
  • 180
  • @πάνταῥεῖ In theory you are right, but for a singleton, there shouldn't exist a second instance to assign to begin with ;) But yes, it is possible to end up with stupid code that accidentally triggers assignment, I'll edit the answer. – Daniel Frey Jun 02 '15 at 18:30
  • @DanielFrey I guess one can do `Singleton::getInstance() = Singleton::getInstance();`, which is really out of bounds, but still doable ;) – vsoftco Jun 02 '15 at 18:31
  • @vsoftco Yes, but that is not an accident anymore, it's asking for trouble. :-D – Daniel Frey Jun 02 '15 at 18:32
  • If it can be done, eventually some one will do it :D But I totally agree, it is asking for trouble. – vsoftco Jun 02 '15 at 18:33
  • 1
    @vsoftco That's why I consider being slightly paranoid to be an asset for a software developer :-P – Daniel Frey Jun 02 '15 at 18:34
3

Try adding a public copy-constructor:

Singleton(const Singleton&) { std::cout << "copy construction!\n"; }

Your output will become:

construction!
copy construction!
foo!
destruction!
destruction!

There are two "singletons" alive because the one from getInstance() got copied. To avoid inadvertently copying singletons, you should delete the copy constructor and assignment operators:

Singleton(const Singleton&) = delete;
Singleton& operator=(const Singleton&) = delete;
isanae
  • 3,253
  • 1
  • 22
  • 47
2

You may hide the singleton in a regular class not exhibiting its static nature:

#include <iostream>
#include <string>

class Singleton // A bad name for a specific class, you should not generalize
{
    private:
    struct Context {
        std::string name;
        Context()
        :   name("Hello")
        {}
    };

    private:
    static Context& context() { static Context result; return result; }

    public:
    const std::string& name() const { return context().name; }
};

int main()
{
    Singleton a;
    std::cout << a.name() << '\n';
}
Lightness Races in Orbit
  • 378,754
  • 76
  • 643
  • 1,055