Shared pointers to a singleton do not recognize each other

Question

I am currently learning how to use the C++11 smart pointers while programming a 2D game engine as a hobby using SDL. However, I ran into a problem while implementing an OOp wrapper for SDL.

The intent is to create a singleton class, which initializes SDL when it is constructed, and shuts SDL down when it gets destroyed. The singleton class has a static method getInstance that returns a shared_ptr to the singleton, and constructs the single instance if no instance exists, the idea being that all clients of the singleton own a shared_ptr to it, and when all clients get destroyed, the singleton is also destroyed. I do understand that singletons (and other globals) are generally bad, but I think that this could be one of the few cases where a singleton is appropriate, as there can only be one SDL library in use.

The problem lies in returning the shared_ptr from the getInstance method. Instead of using the same shared_ptr manager object, the shared_ptr instances are unrelated, and destroying a single one of them deallocates the singleton.

#include <iostream>
#include <memory>
using namespace std;

class Foo
{
public:
    ~Foo(){cout << "Foo <" << this << "> destroyed\n"; instance_ = nullptr;}
    static shared_ptr<Foo> getInstance()
    {
        if(instance_ == nullptr)
            instance_ = new Foo;
        //problem: the shared pointers created are unaware of each other
        return shared_ptr<Foo>(instance_);
    }
private:
    Foo(){cout << "Foo <" << this << "> constructed\n";}
    Foo(Foo& other){}
    void operator=(Foo& other){}
    static Foo* instance_;
};

Foo* Foo::instance_ = nullptr;

int main()
{
    shared_ptr<Foo> a = Foo::getInstance();
    shared_ptr<Foo> b = Foo::getInstance();
    shared_ptr<Foo> c = Foo::getInstance();
}

Output:

Foo <0x3e2a10> constructed
Foo <0x3e2a10> destroyed
Foo <0x3e2a10> destroyed
Foo <0x3e2a10> destroyed

Answer 1

Your method could look like this:

static shared_ptr<Foo> getInstance()
{
    static std::shared_ptr<Foo> instance = std::make_shared<Foo>();
    return instance;
}

That way only a single, static instance is created (since C++11 this is also thread-safe) and you are always returning a copy from the static shared_ptr, which means all shared pointers that are returned share ownership.

Your original attempt created separate instances of shared_ptr from the same plain pointer, but this leads to detached ownership since the different shared pointers have no knowledge of each other and each one has its own internal "shared count".

---

Update: Re-reading your question I think you don't want to extend the life-time until the end of the program. Consider this method to destroy the instance as soon as all the returned shared pointers went out of scope:

static std::shared_ptr<Foo> getInstance()
{
    static std::weak_ptr<Foo> instance;
    static std::mutex mutex; // (a)
    const std::lock_guard< std::mutex > lock( mutex ); // (b)
    if( const auto result = instance.lock() ) return result;
    return ( instance = std::make_shared<Foo>() ).lock();
}

where you could delete the lines marked (a) and (b) if you are not in a multi-threaded environment.

Note that this is actually a so-called phoenix-singleton, meaning that if all shared pointers you received from getInstance() went out of scope and the instance was deleted, the next call to getInstance() will create another new instance of Foo.

Answer 2

Rather than keeping a raw pointer in your static member, you should keep a weak_ptr. Use the lock() function to convert back to a shared_ptr; if it comes back empty then you need to allocate a new singleton object.

    static shared_ptr<Foo> getInstance()
    {
        shared_ptr<Foo> p = instance_.lock();
        if (!p)
        {
            p = new Foo;
            instance_ = p;
        }
        return p;
    }
private:
    static weak_ptr<Foo> instance_;