c++ singleton implementation STL thread safe

Question

I've been trying to implement a singleton with some C++11 features from STL. I read about a few implementations and I found this pretty good: http://silviuardelean.ro/2012/06/05/few-singleton-approaches/ I made a few changes and got the bellow code working on VS2013, but I still would like to know:

a) Is this implementation thread-safe?

b) Is it ok (good practice) to return a shared_ptr from GetInstance instead of a reference?

PS.: My singleton is ment to be an OpenGL interface (that's the reason for the name).

HandlerOpenGL.h

#pragma once



// STL headers
#include <memory> // shared_ptr
#include <mutex> // once_flag



// OpenGL Handler Singleton
class HandlerOpenGL
{
public:
    // Constructor/Destructor interface
    static std::shared_ptr<HandlerOpenGL> GetInstance(void);    // Only way to access singleton
    ~HandlerOpenGL(void);
    HandlerOpenGL(const HandlerOpenGL&) = delete;   // Avoid any copy/move
    HandlerOpenGL(HandlerOpenGL&&) = delete;
    HandlerOpenGL& operator=(const HandlerOpenGL&) = delete;
    HandlerOpenGL& operator=(HandlerOpenGL&&) = delete;

private:
    // Hide construction method/variables
    HandlerOpenGL(void);    // Private to be created once
    static std::shared_ptr<HandlerOpenGL> _instance;
    static std::once_flag _flag;
};

HandlerOpenGL.cpp

// Own header
#include "HandlerOpenGL.h"
// STL headers
#include <memory> // shared_ptr, make_shared
#include <mutex> // once_flag, call_once



// instanciate static members
std::shared_ptr<HandlerOpenGL> HandlerOpenGL::_instance(nullptr);
std::once_flag HandlerOpenGL::_flag;



std::shared_ptr<HandlerOpenGL> HandlerOpenGL::GetInstance(void)
{
    std::call_once(_flag, [](){ _instance.reset(new HandlerOpenGL); });
    return _instance;
}



HandlerOpenGL::~HandlerOpenGL(void)
{
}



HandlerOpenGL::HandlerOpenGL(void)
{
}

Article you linked has one serious problem: it claims that **static variables are not thread-safe. This is false**. In C++11 initialization of static-variables is required to be thread-safe. So all those manipultion with `call_once` are not needed. — Revolver_Ocelot, Dec 24 '15 at 16:57
Thanks a lot! I read this article: http://www.aristeia.com/Papers/DDJ_Jul_Aug_2004_revised.pdf that claimed static variables weren't thread safe, but it's from 2004. — etlourenco, Dec 24 '15 at 17:11

score 2 · Answer 1 · answered Dec 24 '15 at 16:55

2

I do not see a point of using shared_ptr there at all. If it is a singleton, it will not be copied. So why use shared_ptr?

I also believe, a Meyers singleton is so much easier to do, requires less typing, and doesn't rely on dynamic allocation, so I wonder why would anybody would do anything else.

Despite all this, I do not see a specific threading issues with that.

answered Dec 24 '15 at 16:55

SergeyA

61,605
5
78
137

I also think Meyers singleton is easier (and cleaner) but I thought it wasn't thread-safe due to static variables. Now I see: http://stackoverflow.com/questions/1661529/is-meyers-implementation-of-singleton-pattern-thread-safe – etlourenco Dec 24 '15 at 17:21
Yes, Meyer's singleton in C++2011 and up is 100% thread safe. – SergeyA Dec 24 '15 at 17:42

score 1 · Answer 2 · edited May 23 '17 at 12:15

1

I think using static variable at member function better than static member. This "_instance" will be created once method called.

HandlerOpenGL& HandlerOpenGL::GetInstance(void)
{
    static HandlerOpenGL _instance;
    return _instance;
}

look at this

edited May 23 '17 at 12:15

Community

1
1

answered Dec 24 '15 at 17:41

wow2006

311
3
6

score 0 · Answer 3 · answered Dec 24 '15 at 18:32

Pro tip - if you do think you need a singleton, give it value semantics and encapsulate its singleton nature as a private implementation detail.

In the future you may realise you don't need a singleton after all. If you wrote the class with value semantics, you wont have to change any user code - only the private implementation of the singleton:

struct HandlerOpenGL
{
    // public interface

    // note - allow it to be copyable.

    // interface exists as instance method
    void do_some_graphics();

private:
    struct impl;
    // this line is the only clue that singletons are involved
    static auto get_impl() -> impl&;
};

// implementation (private)

struct HandlerOpenGL::impl
{
    // this class holds your static data such as library pointers
    // it can be non-moveable etc

    impl()
    {
        // _gfx_engine = init_gfx_engine();
        // if (!_gfx_engine)
        //   throw std::runtime_error("gfx engine failed to init");
    }

    ~impl()
    {
        // commented out so that this example compiles with no
        // external dependencies
        // if (_gfx_engine)
        //   deinit_gfx_engine(_gfx_engine);
    }

    // gfx_engine * _gfx_engine;

};

// this is the private singleton-fetcher    
auto HandlerOpenGL::get_impl() -> impl&
{
    static impl _;
    return _;
}

// implement the interface

void HandlerOpenGL::do_some_graphics()
{
    // fetch my internal singleton
    auto& static_data = get_impl();

    // use methods and data in static_data here
    // gfx_draw_surface(static_data._gfx_engine);
}

// now you can use it like a value - and it's decoupled from logic

// a function that renders graphics on any appropriate graphics engine
template<class T>
void some_graphic_render(T handler)
{
    handler.do_some_graphics();
}

int main()
{
    auto gfx = HandlerOpenGL();   // created trivially - behaves like a value

    auto gfx2 = gfx;    // copies no problem at all

    // use as an argument to allow ADL - allows decoupling and dependency injection

    some_graphic_render(gfx);
    some_graphic_render(HandlerOpenGL());   // can even create as needed. no performance cost

    return 0;
}

c++ singleton implementation STL thread safe

3 Answers3

Linked