CODEWITHSUNDEEP
c++c++11singletonlazy-loadingunique-ptr
cavitsinadogru
cavitsinadogru

Reputation: 1030

static unique_ptr calls destructor twice

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.

Upvotes: 6

Views: 13198

Answers (3)

Peter
Peter

Reputation: 21

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();

Upvotes: 2

bone
bone

Reputation: 154

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.

Upvotes: 5

Rakete1111
Rakete1111

Reputation: 48948

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();

Upvotes: 12

Related Questions