6

I use a singleton pattern which returns a reference to unique_ptr dereference. Here is the code,

#include <iostream>
#include <memory>
using std::cout; using std::endl;
using std::unique_ptr;

namespace Settings {
    class Lazy {
        Lazy() { cout << "Lazy::Lazy() " << this << endl; }
    public:
        ~Lazy() { cout << "Lazy::~Lazy() " << this << endl; }
        static Lazy &instance()
        {
            static unique_ptr<Lazy> lazy(new Lazy);
            return *lazy;
        }
    };

    Lazy &lazy()
    { return Lazy::instance(); }
}

int main()
{
    cout << "main starts!" << endl;
    auto state = Settings::lazy();
    cout << "auto state = Settings::lazy() " << &state << endl;

    cout << "main ends!" << endl;
    return 0;
}

I was expecting that the destructor of the class would call only once but although the constructor called once destructor called twice, here is the output,

main starts!
Lazy::Lazy() 0xb1ec20
auto state = Settings::lazy() 0x7ffe17ae18b8
main ends!
Lazy::~Lazy() 0x7ffe17ae18b8
Lazy::~Lazy() 0xb1ec20

why destructor called twice? And even the second call this address is different.

cavitsinadogru
  • 940
  • 2
  • 10
  • 17
  • 2
    `auto state = Settings::lazy();` calls the automatic copy constructor that you don't have any trace output inside. Create a private one if you really want a singleton, and you'll see the error. – Joachim Isaksson Mar 26 '16 at 21:31
  • Why use a `unique_ptr`? Might as well just have a static object. – Galik Mar 26 '16 at 21:34
  • 1
    I recommend looking at this answer: http://stackoverflow.com/a/1008289/3807729 – Galik Mar 26 '16 at 21:37
  • Also you should really put your `using` declarations *after* all the *includes* because you are making those symbols visible to the header files that come after the declarations. – Galik Mar 26 '16 at 21:39
  • @Galik Ah thanks for this `using` declarations tip! But for your first comment, if I make the object static wouldn't be this constructed before the main starts? My purpose using `unique_ptr` was to make lazy-loading. So that object would not be instantiated until I need it. – cavitsinadogru Mar 26 '16 at 21:45
  • 2
    No, in a function a static is not constructed until the function is called for the first time. Also the construction of the static object is guaranteed to be *thread safe* for the first call (while the static objects construct). – Galik Mar 26 '16 at 21:46

3 Answers3

11

Because you have 2 instances of the singleton, and both get destroyed.

The reason why you have to 2 singletons, is that when you get the singleton auto state = Settings::lazy(); a copy is created. You might be returning a reference, but state isn't a reference, so a copy is created.

Making state a reference fixes the problem: auto& state = Settings::lazy();

Rakete1111
  • 47,013
  • 16
  • 123
  • 162
  • Also see https://stackoverflow.com/questions/7138588/c11-auto-what-if-it-gets-a-constant-reference for deducing value vs reference type with `auto` – hsandt Mar 18 '19 at 12:40
4

Rakete1111 solution is correct, but you might also want to delete the copy constructor and copy assignment for your singleton. That way you would prevent errors like this from ever happening.

bone
  • 144
  • 2
  • 6
1

Singleton class must disable any copy/move constructors & assignment/move operators. Otherwise this is not a singleton.
Thus,

Lazy(Lazy &) = delete;
Lazy(Lazy &&) = delete;
Lazy &operator=(Lazy &) = delete;
Lazy &operator=(Lazy &&) = delete;

Then the following will be the only valid operation:

auto& state = Settings::lazy();
Peter
  • 11
  • 1