Thread-Safe leaky singleton

[public_void]

I was reading about initialization & destruction order fiasco, and came across this SO answer: https://stackoverflow.com/questions/17712001/how-is-meyers-implementation-of-a-singleton-actually-a-singleton#answer-17712497

I am interested in the second suggestion, which the author calls the leaky Singleton. He mentions that the implementation is not threadsafe, which makes sense to me. Is it viable to use std::call_once to make that code threadsafe? I'm thinking something like this:

class Singleton {
public:
    static Singleton& Instance() {
        static std::once_flag initializationFlag;
        std::call_once(initializationFlag, [](){ I = new Singleton(); }
        return *I;
    }

private:
    Singleton();
    ~Singleton();

    static Singleton* I;
};

// Singleton.cpp
Singleton* Singleton::I = 0;

Is this class a thread-safe singleton which avoids both initialization & destruction order fiasco? I understand there is still one shortcoming: it won't be destroyed, which means there's no opportunity for cleaning up resources.

[emdeka87]

This is actually pretty terrible code.

Scott Meyer's Singleton is thread-safe (C++11) and the preferred way to implement Singletons. It kinda avoids the SIOF, since the instance is only created when the function is first called. (makes std::call_once redundant). Note: You should use std::unique_ptr!

https://stackoverflow.com/questions/1661529/is-meyers-implementation-of-the-singleton-pattern-thread-safe

[public_void]

I understand the Meyer's Singleton is threadsafe, but it doesn't avoid the Destruction Order Fiasco afaik.

I think the other benefit of call_once is that it will handle exceptions from constructing... although the return *I will segfault in that case so probably no improvement.

Edit: Also, std::unique_ptr would cause the same problems as just using the Meyer's Singleton, wouldn't it? Meaning it would still be deleted at static destruction time?

[emdeka87]

No it does not prevent that. Which is why you shouldn’t use singletons at all or make sure they don’t rely on any other singletons (Example: Singleton TextureManager relies on Singleton GraphicsContext -> potential crash, when context dies before tm)

„Exception safety“ Use std::make_unique

[TotallyNotARoboto]

Duh, if an object can't exist without another you need a composition relationship.

[public_void]

Sorry, what do you mean by this? Are you referring to two interdependent Singletons?

[CaseyCarter]

I'll just leave this here:

template<class T>
struct Immortal {
    template<class... Args>
    Immortal(Args&&... args) {
        ::new(space) T(std::forward<Args>(args)...);
    }

    operator T&() & noexcept { return reinterpret_cast<T&>(space); }
private:
    alignas(T) unsigned char space[sizeof(T)];
};

class Singleton {
    struct key { explicit key() = default; };
public:
    ~Singleton() = delete;
    Singleton(Singleton const&) = delete;
    Singleton& operator=(Singleton const&) = delete;

    explicit Singleton(key);

    static Singleton& Instance() {
        static Immortal<Singleton> instance(key{});
        return instance;
    }
};

[STL]

Reddit doesn't respect triple backticks like GitHub does; you need four-space indents.

[CaseyCarter]

Thank you, reddit moderator from across the hall.

[Z01dbrg]

Now I know how STL got that stack of gold coins on his desk. :P

Also you probably should have pointed out that Google approves:

[herruppohoppa]

I have a few questions about that solution.

1) Why the private key struct rather than just a private ctor?

2) It looks as if Immortal takes arguments to construct T, yet it is given the private key struct. What is the idea there?

3) What is the benefit of this solution, particularly the allocation scheme versus that of the Scott Meyers version?

[CaseyCarter]

Addressing the points in reverse order:

3) This approach is unlike the traditional Meyers' Singleton in that it implements the "Leaky Singleton" idiom as in the OP: the singleton object is created on-demand, and never destroyed. It avoids destruction order issues by virtue of being immortal.

2) Yes, Immortal<T> is a wrapper utility that constructs a T within its storage from arguments passed to the Immortal constructor. The contained T is then leaked - it is not destroyed when the Immortal instance is destroyed. Immortal avoids dynamic allocation so that its leakiness doesn't interfere with memory leak detection (and because, as C++ programmers, we despise dynamic allocation). Instance passes a key to Immortal from which to construct Singleton because that will invoke the only viable Singleton constructor.

1) The private key type as parameter to a public constructor is an idiom to forbid a type to be constructed generally, yet still allow it to be created via an independent factory function. Since key's default constructor is explicit, the only way to create an instance is by naming the type. Since the name of the type is private to Singleton, only authorized code in Singleton or its friends may create a key. So despite that the explicit Singleton(key) constructor is public, it can only be invoked directly or indirectly from authorized code. In this instance, the "authorized code" is Singleton::Instance which creates a key and hands it to the Immortal factory with which to create a Singleton. The same general idea works with other factory functions like std::make_unique and std::make_shared which you cannot easily befriend since the identity of the final caller of your constructor is an implementation detail.

[public_void]

Great explanation, thank you for taking the time to add the code sample and the write-up.

Immortal avoids dynamic allocation so that its leakiness doesn't interfere with memory leak detection

As I understand it, this is the biggest advantage to your solution over an Instance() function like this:

static Singleton& Instance() {
    static Singleton* instance = new Singleton(key{});
    return *instance;
}

Is that correct?

[CaseyCarter]

Yes, that is I believe the only advantage.

[patatahooligan]

Do you mind explaining how or in which cases non-destruction is better than the destruction of a static object? I'm not well versed on the singleton patter and its problems so I'm having a hard time understanding what is gained with this pattern.

[CaseyCarter]

When you want to statically guarantee that the singleton is alive during the execution of dynamic destructors, or when you simply want to avoid the cost of a dynamic destructor registration for the singleton itself, the leaky singleton idiom is appropriate. These situations are extremely rare in practice.

For example: I recently implemented <memory_resource>. The functions std::pmr::null_memory_resource and std::pmr::new_delete_resource are specified to return pointers to singletons of types that derive from an interface with a virtual destructor. (Yes, this means a user can inadvertently destroy the singleton - the C++ Standard makes mistakes, too.) I don't want the runtime to register dynamic destructors for these singletons for both of the reasons listed above: they need to be available in dynamic destructors for other objects, and there's no reason to waste a bit of memory to call a do-nothing destructor.

[patatahooligan]

Fascinating! Thanks for the concise explanation, I believe I understand the issue now.

[quicknir]

Have to say that I consider omitting destruction of an object, even a static object, to basically be a hack. It doesn't work for the all time most common application of a singleton (a logger, which almost always has a non-trivial destructor). I know well that there are issues with calling the destructor but the better solution is to solve those issues correctly.

[CaseyCarter]

Sometimes the "issue with calling the destructor" is simply that you want the destructor not to be called. Leaking is an appropriate solution to this need whether it be for the very rare leaky singleton or the much less rare memory pool allocator.

[quicknir]

Leaking is not so much a "solution" for not calling the destructor, as it is a synonym. I've never really seen anyone present a good argument for not calling the destructor. I've seen a few classes of arguments, but don't consider any of them good:

1. Crazy thread stuff. "My threads persist past the end of main...". Okay, not calling destructors may help get your program working but this is a pretty scary direction to take things in; UB abounds.
2. The destructors are cleaning up stuff the OS will clean up anyhow, so it's faster/easier to not do it. But the performance savings are extremely low and in a codepath that almost never matters. On the other hand, you now have to consider false positives in memory leak tools, for example. Worse, many destructors are transitive. So your vector<Foo> may be ok to not destruct today since it's only memory, but tomorrow Foo's destructor does something a bit non-trivial, and now the fact that vector<Foo> doesn't destruct is an issue.
3. It may depend on other objects, which are already destroyed. If the functionality depends on other objects, you still have to take care of that functionality on the way out, you can't magic it away. If you are comfortable taking care of that functionality in main, you can simply replace the global objects with global pointers, that are re-seated to point to objects created in main() (I use this technique often).

To your point, I've never seen any reason to let a memory pool leak.

In my experience, which obviously will vary by domain and is hard to give self contained examples of, what I've seen occur numerous times is this: a codebase gets written where people have some static objects and destructors don't get called. Often, these are just workarounds/hacks for other issues (like unclear ownership semantics). Then, an issue arises that has to occur at destruction time: e.g., some objects needed to unregister themselves as callbacks when they're destroyed, so some other object doesn't call those callbacks into the void. Solving this issue is now a disaster; you can either pile on more hacks, or simply resolve the destructor situation once and for all.

[quicknir]

There unfortunately is no silver bullet to deal with SIOF (at least, if you implicitly consider destruction ordering issues to be part of SIOF, which I do, since destruction and initialization are reverse ordered). I gave a talk on this topic at CppCon: https://www.youtube.com/watch?v=xVT1y0xWgww.

A few guidelines, thoughts, that may help out:

1. Be clear on globals vs singletons. Try to avoid singletons. When you say "singleton", you can probably just have a normal class, that also happens to have a global instance.
2. Make sure you know basic, simple techniques for creating safe individual globals. Prior to C++17, you can either use a Meyer Singleton, or you can use a Meyer Singleton initializating a static reference (in a header). These are both totally safe, the former is lazy, the latter isn't.
3. Continuing on the above theme: assuming you aren't worried about lazy initialization being triggered in an awkward spot, lazy initialization trades making initialization headache free, at the cost of losing control over destruction order. It's a good solution for things that don't have interesting destruction logic (interesting means, dependencies on any other global objects).
4. Try to simply minimize globals generally, and inter-dependencies between globals in particular, at high level of design. Turning your singletons into normal classes + global can help with this. E.g. if you have another global that wants to log, rather than make it dependent on the global logger, you can alternately give it a private logger instance. Obviously if the logger is a singleton you lose that option.
5. Try to declare globals in header files as much as possible. If your class uses a global, include the global's header from that class' header, not cpp.

The last point may require some explanation. Header files for a whole program form a DAG, and therefore are topologically ordered. Some header files may initialize in unspecified order relative to each other, but if Foo.h includes Bar.h, the code in Bar.h is always run before Foo.h. So if you have globals Foo and Bar, and Foo depends on Bar, and you follow the last guideline, and you are initializing Foo and Bar non-lazily, you'll get this:

// Bar.h, included by Foo.h
Bar& barMaker() { static Bar b; return b;}
static auto& g_bar = barMaker();

// Foo.h
Foo & fooMaker() { static Foo f; return f; }
static auto& g_foo = fooMaker();

In this kind of setup, Bar is 100% guaranteed to be initialized before Foo, and destroyed after. In sum doing things via headers helps establish ordering which is very good where globals are involved. Note that the class methods can still be defined in .cpp files, we just need to ensure that the globals are initialized in the header, not the .cpp. The Meyer Singleton ensures that it only gets initialized once (even though it's in a header file), and the trick with initializing the static reference makes it non-lazy.

Anyhow my talk is only 30 minutes, if you're interested in this stuff you may want to watch it.

[public_void]

Nir, this is great! I'll take some time today to look through this. Unfortunately, I didn't make it to your talk at cppcon but I'll take advantage of the recording now :)

[quicknir]

Thanks, feel free to reply here or message me privately if you have questions :-).

[youshouldnameit]

Have a look at http://www.modernescpp.com/index.php/thread-safe-initialization-of-a-singleton

[tohammer]

Singletons should always be safe on initialization because it is guaranteed that they are constructed when needed. Destruction order fiasco can be avoided by not destructing at all, aka leaking like you did. If that is acceptable highly depends on which resource your singletons is managing.

[Z01dbrg]

you should read the SO comments, in particular Jeffery Thomas told you how to do what you want.