2

So I've written a simple singleton class. When I create an object the constructor is called, but it's destructor (which frees the object) doesn't seem to be called when it goes out of scope.

#include <iostream>

using namespace std;

class Singleton {
public:
    static Singleton &getInstance( )
    {
        if (instance == nullptr) {
            cout << "Creating instance.\n";
            instance = new Singleton();
        }
        return *instance;
    }
    static void destroyInstance( )
    {
        if (instance != nullptr) {
            cout << "Destroying instance.\n";
            delete instance;
            instance = nullptr;
        }
    }
    ~Singleton( )
    {
        if (instance != nullptr) {
            cout << "Destroying instance.\n";
            delete instance;
            instance = nullptr;
        }
    }
private:
    Singleton( ) { }

    static Singleton *instance;
};
Singleton *Singleton::instance = nullptr;

int main( )
{
    Singleton &singleton = Singleton::getInstance();

    //singleton.destroyInstance();

    return 0;
}

With the destructor code the program just outputs this.

Creating instance.

If I comment out the destructor and use the destroyInstance() function it outputs this.

Creating instance.
Destorying instance.

Why is it doing this?

EDIT: I'm an idiot. Just overlooked a bunch of things. This question might as well be deleted as it's not really that useful.

  • 2
    I'm confused. What mechanism were you expecting to come into play to `delete` your singleton if you do not call `delete` on it? Implementing a destructor simply describes what happens when an instance is destroyed, it does not have any impact on *if* or *when* it is destroyed. – François Andrieux Apr 19 '17 at 19:46
  • Your destructor is going to fail at least once. – Captain Obvlious Apr 19 '17 at 19:47
  • 1
    Pointers don't get deleted when they go out of scope. Normally a singleton is initialized once anyway. Calling `delete` on yourself inside your own destructor is also probably a super bad idea. – tadman Apr 19 '17 at 19:49
  • @FrançoisAndrieux I don't really know. From what I understand it's just good practice to delete things allocated with `new`. Though I will still use the destructor for other things. –  Apr 19 '17 at 19:50
  • @TomSmale *So I've written a simple singleton class* -- Not as simple as the [Meyer's singleton](http://stackoverflow.com/questions/17712001/how-is-meyers-implementation-of-a-singleton-actually-a-singleton) – PaulMcKenzie Apr 19 '17 at 19:51
  • @JesperJuhl Oh I see. So the destructor will not get called on a singleton untill the end of the program. I think I've overlooked the attributes of the static keyword. –  Apr 19 '17 at 19:53
  • @TomSmale To be clear, with the code you showed (with destroyInstance commented out), the destructor never runs, not at the end of the program either. The end of the program can't magically run destructors (it will however reclaim most resources, memory, sockets, etc). – Nir Friedman Apr 19 '17 at 20:04
  • @TomSmale A pointer reaching the end of it's lifetime does not automatically cause the deletion of the pointed object. You *must* call `delete` yourself on every instance you allocate with `new` (it's not just good practice). Destructors are called when an instance is destroyed, for example when you call `delete` yourself. They are in no way a replacement for `delete` and preform very different duties than `delete`. – François Andrieux Apr 19 '17 at 20:04
  • @TomSmale You may be interested in [`std::unique_ptr`](http://en.cppreference.com/w/cpp/memory/unique_ptr) and [`std::shared_ptr`](http://en.cppreference.com/w/cpp/memory/shared_ptr). – François Andrieux Apr 19 '17 at 20:05
  • You have no automatic duration `Singleton`, so you shouldn't expect the destructor to run automatically. The pointer itself is destroyed, but not what it points to; that's what `delete` is for, it lets you call the pointed-to object's destructor and deallocate the memory itself in one fell swoop. As François Andrieux said, you might want to look into `std::unique_ptr`; it would let you do [something like this](http://rextester.com/UOFG36197). (The destructor does nothing because the `unique_ptr`'s destructor will automatically delete the pointer for you.) – Justin Time - Reinstate Monica Apr 19 '17 at 20:35

2 Answers2

5

The logic in your destructor is weirdly recursive:

~Singleton( )
{
    if (instance != nullptr) {
        cout << "Destroying instance.\n";
        delete instance;
        instance = nullptr;
    }
}

You call delete on the instance in the destructor, but calling delete is what triggers the destructor to run!

You say that the destructor doesn't get called when it goes out of scope, but remember that you created your singleton with new; such objects do not have their destructor called automatically any more when scope ends.

Unless you really desperately need to destroy and re-create your singleton in the middle of the program (extremely rare in my experience), just create your singleton like this instead:

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

Get rid of destroyInstance, and just have the destructor clean up after itself as it should, without worrying about the static instance.

Nir Friedman
  • 17,108
  • 2
  • 44
  • 72
0

Using smart pointer (#include <memory>)

#include <iostream>
#include <memory>

using namespace std;

class Singleton {
public:
    static Singleton &getInstance()
    {
        if (instance == nullptr) {
            cout << "Creating instance.\n";
            instance = std::shared_ptr<Singleton>(new Singleton());
        }
        return *instance;
    }
    static void destroyInstance()
    {
        if (instance != nullptr) {
            cout << "Destroying instance.\n";
            instance.reset();
            instance = nullptr;
        }
    }
    ~Singleton()
    {
        if (instance != nullptr) {
            cout << "Destroying instance.\n";
            instance.reset();
            instance = nullptr;
        }
    }
private:
    Singleton() { }

    static std::shared_ptr<Singleton> instance;
};
std::shared_ptr<Singleton> Singleton::instance = nullptr;

int main()
{
    Singleton &singleton = Singleton::getInstance();

    //singleton.destroyInstance();

    return 0;
}