2
\$\begingroup\$

I want to implement the singleton pattern in a header only c++ library. (Please refrain from sharing your opinion on the pattern itself!) This would be trivial if I could rely on the compiler implementing thread safe c++11 function-local static-duration object initialization. i.e.

static inline MyClass& singleTon()
{
    static MyClass instance;
    return instance;
}

Unfortunately MSVC prior to 2015 does not comply with this standard. See "Magic Statics" so I have tried to implement it through a class:

#include <mutex>
/**
 * Header only implementation of the singleton pattern.
 * Use like this: 
 * MyClass& singleton = SingleTon<MyClass>().get();
 */
template<class Type>
struct SingleTon
{
    static std::unique_ptr<Type> instance;
    static std::once_flag flag;
    Type& get()
    {
        auto& capture = instance;// lambdas can't capture static members
        std::call_once(flag, [&capture] { capture.reset(new Type{}); });
        return *instance.get();
    }
};

template<class Type>
std::unique_ptr<Type> SingleTon<Type>::instance;

template<class Type>
std::once_flag SingleTon<Type>::flag;

So my function now looks like:

static inline MyClass& singleton()
{
    return SingleTon<MyClass>().get();
}

I hope that this function is thread safe given that the ctor of Typeis nothrow.

\$\endgroup\$
  • \$\begingroup\$ Did you test it? Does it work as intended? \$\endgroup\$ – Mast Nov 18 '16 at 11:27
  • \$\begingroup\$ Yes. AFAICT ... \$\endgroup\$ – bgp2000 Nov 18 '16 at 12:47
  • 2
    \$\begingroup\$ Do not edit the question after you have received an answer as it invalidates the answer. \$\endgroup\$ – Emily L. Nov 18 '16 at 13:12
2
\$\begingroup\$

I have a problem with this because

template<class Type>
struct SingleTon
{
    static std::unique_ptr<Type> instance;

This is still a static storage duration object. This means its initialization can happen after a call to Type& get(). So you have to solve this problem first.

So the instance variable needs to be wrapped in a function to make sure initialization of instance is guaranteed. If you are going to do that you may as well not dynamically create the object.

How about this:

template<class Type>
struct SingleTon
{
    private:
        static Type& instance()
        {
            // Use static function scope variable to 
            // correctly define lifespan of object.
            static Type instance;
            return instance;
        }
    public:
        static Type& get()
        {
            // Note the constructor of std::once_flag
            // is a constexpr and is thus done at compile time
            // thus it is immune to multithread construction
            // issues as nothing is done at runtime.
            static std::once_flag flag;

            // Make sure all threads apart from one wait
            // until the first call to instance has completed.
            // This guarantees that the object is fully constructed
            // by a single thread.
            std::call_once(flag, [] { instance(); });

            // Now all threads can go get the instance.
            // as it has been constructed.
            return instance();
        }
};

The Elephant in the room (Linker)

Now that we solved this issue. There is a larger elephant in the room to solve. (which is also true for the original code).

Since this is a template class with its whole definition in the header file there will be several versions of this function spread around the code (one in each compilation unit). It becomes the job of the linker to remove all but one copy. If you have a simple application that just uses object files this does not seem to be an issue.

BUT I have come across linkers that were unable to do this when the object were compiled into separate dynamically loaded libraries. Thus in effect your singleton becomes a singleton per runtime library that uses it. This is why you don't find Singelton libraries defined like this with a template class as the harness.

\$\endgroup\$
  • \$\begingroup\$ Good point about the potential 'static initialization fiasco'. I have two questions about that: 1. Since unique_ptr also has a constexpr constructor, wouldn't it be enough to just pull flag and instance into function-local static scope and be done with it? 2. Are you sure static members with constexpr constructors aren't initialized at compile time? \$\endgroup\$ – bgp2000 Nov 18 '16 at 18:55
  • \$\begingroup\$ constexpr constructors allow const initialization \$\endgroup\$ – Martin York Nov 18 '16 at 20:46
  • \$\begingroup\$ I did not notice that std::unique_ptr was contexpr initilazable. So yes you could. But why would you want to to introduce dynamic allocation into the mix. \$\endgroup\$ – Martin York Nov 18 '16 at 20:47
2
\$\begingroup\$

I have also encountered the lack of magic statics for MSVC < 2015.

I initially used a similar solution (using call_once and a unique_ptr) but it encountered a bug in MSVC 2013.

When Type contained a std::thread that was not joined and the unique_ptr is destroyed on program shutdown, then the destructor of std::thread will hang waiting to join the thread which will never happen and the program remains alive.

This is a known bug: std::thread::join() hangs if called after main() exits when using VS2012 RC

My solution to this was to simply leak the Type instance as there was no way of safely destructing it and just let the OS deal with it.

As for your code, it's spelled "Singleton" not "Single Ton". Also use a class instead of a struct and make the members private. Otherwise Singleton<Foo>::instance can be used instead of Singleton<Foo>().get() which you intend.

I would also make get static so that you just need to call Singleton<Foo>::get().

\$\endgroup\$
  • \$\begingroup\$ Thanks for the warning about join()! Is this problem related to call_once(), or would you encounter it with static members as well? I think I would argue that it is an error by the programmer not to join all threads before main returns, or am I missing something? I've incorporated the rest of your suggestions. \$\endgroup\$ – bgp2000 Nov 18 '16 at 12:46
  • \$\begingroup\$ It would be encountered by static members as well. It is not a programmer error. The programmer is wholly correct in expecting the destructor of a static to be ran on program shutdown and do the joining in said destructor. One can argue whether or not that is good design but it is not a programmer error. \$\endgroup\$ – Emily L. Nov 18 '16 at 12:47
  • \$\begingroup\$ Ok, error is a bit harsh, but the problem is orthogonal to the implementation of this class then. \$\endgroup\$ – bgp2000 Nov 18 '16 at 12:49
  • \$\begingroup\$ Depends on how you look at it. You class will cause the program to hang on exit if Type contains an unjoined std::thread. Type can be correctly implemented and the program would still hang, hence I argue the problem is this class not correctly working around that bug as well. But that is up to you really. \$\endgroup\$ – Emily L. Nov 18 '16 at 12:55
  • \$\begingroup\$ Well, you said "It would be encountered by static members as well". I don't see how any user of the Singleton class could reasonably expect it to fix a problem they would encounter with any kind of static allocation. That's what I meant by orthogonal. Thanks for bringing the bug to my attention, anyway! \$\endgroup\$ – bgp2000 Nov 18 '16 at 12:59

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.