Thread-safe singleton class using std::shared_ptr in C++(11) What I am trying to accomplish is to create an efficient thread-safe singleton base class (as stated in the title).

So this is my singleton class, which is used via inheritance (see below).

#pragma once
#ifndef SINGLETON_HEADER_INCLUDED
#define SINGLETON_HEADER_INCLUDED
#include &lt;memory&gt;

template &lt;class T&gt;
class singleton
{
public:
        static T *get_instance(void)
        {
                if(instance.get() == 0 || instance == (NULL &amp;&amp; 0) || !instance) {
                        instance = std::shared_ptr&lt;T&gt;(new T());
                }
                return instance;
        }

        static delete_instance(void)
        {
                delete instance;
                instance = (NULL &amp;&amp; 0);
                return;
        }

protected:
        singleton(void);
        virtual ~singleton(void);

private:
        singleton(const singleton&amp;);
        singleton&amp; operator = (const singleton&amp;);

        static volatile std::shared_ptr&lt;T&gt; instance;
};

template &lt;class T&gt; template *singleton&lt;T&gt;::instance = 0;

#endif


Here is the class in use:

#include "singleton.hpp"

class opengl_renderer : public singleton&lt;opengl_renderer&gt;
{
        friend class singleton&lt;opengl_renderer&gt;;
        etc...
};


So basically, I am looking for some tips and reviews on my class -> I feel as if it isn't working the way I imagine it should. But it is functional at its current state.
Your code doesn’t compile, and if it would compile, it would crash.

You are trying to assign a shared_ptr to a raw pointer. You are also trying to delete a shared_ptr and this leads me to believe that you don’t understand what a smart pointer actually does.

In fact, your code contains a multitude of errors. More on that below.

First, about the use of shared_ptr. In fact, a shared pointer denotes shared ownership, and this is patently not the case here: the singleton is the owner, nobody else.

A shared pointer is inappropriate here, what you want is a unique_ptr.

Then, don’t expose a pointer to the user, expose a reference via get_instance (i.e. habe get_instance() return T&amp;. And I would rename get_instance to plain instance (which is more C++-y), and rename the private field to something else.

Finally, reconsider your usage of a singleton in the first place. The singleton is generally considered an anti-pattern and real, legitimate uses are so rare that it doesn’t pay to have a singleton base class.

Detailed code critique

if(instance.get() == 0 || instance == (NULL &amp;&amp; 0) || !instance) {


This is doing the same check three times. Why? Once is enough – the latter is the correct usage.

But this code isn‘t thread-safe anyway, in fact, it’s not making any effort to be thread-safe at all.

return instance;


As already said, you cannot cast a shared_ptr to a plain pointer.

static delete_instance(void)


That doesn’t compile, it’s missing a return value. Also, who is going to call this method? It should never be called probably, so just delete it.

delete instance;


As mentioned above, this doesn’t work, and is meaningless. The whole point of a smart pointer is that you don’t need manual deletion.

instance = (NULL &amp;&amp; 0);


This code makes no sense. What is it supposed to do?

virtual ~singleton(void);


You have declared the destructor but you haven’t defined it. The implementation will be empty but you still need to define it.

static volatile std::shared_ptr&lt;T&gt; instance;


volatile probably isn’t meaningful here. In particular, its meaning is unrelated to cross-thread access (unfortunately). But even if it was related to cross-thread access, this wouldn’t make your code thread-safe, it would simply mean that threads would read the most up-to-date version of the variable.

friend class singleton&lt;opengl_renderer&gt;;


What do you need this line for?