Singleton with template wrapper

So far, I've used this in a few places and it seems solid.

This is what I usually do to make a singleton:

```
// Assume x86 for now
#ifdef _MSC_VER
#include // for _mm_pause
#else
#include // for _mm_pause
#endif

#include
#include
#include
#include

// When thrown by get(), this indicates the singleton failed to initialize.
//
// It is better to devise a more elaborate set of exceptions for diagnostic purposes
// (e.g. If it must be initialized in the main thread only, etc...).
class bad_singleton : public std::exception{};

template
class singleton
{
// Should start life as false, no initialization logic needed
// (unless working with a brain-dead compiler, in which case
// we're screwed anyway).
static bool _bad_singleton;

// This should also be 0 on start, but some platforms might require
// a constructor that could fudge things if it runs while the lock is
// being used (meaning that the platform requires kernel objects for all atomic
// operations). To make this robust, use a plain uintptr_t here and
// OS-provided atomic functions (e.g. _InterlockedExchange[64]), If atoms aren't
// supported, you shouldn't be using threads anyway.
static std::atomic _spinlock;

// Once again, shared_ptr might require initialization logic
// that could run after the allocation in get() [assuming we have things
// running before main]. To make this robust, use a pointer here
// and OS-provided atomic functions.
static std::shared_ptr _handle;

public:

static std::shared_ptr get()
{
// Assumes acquire semantics on read.
if(_handle){
// _handle is nonzero? Instant return!
return _handle;
}else{
// Every thread that found _ptr to be null will end up here
// and spin until the lock is released.
while(_spinlock.exchange(1))
{
// One can use system calls here (like Sleep(0) on Windo

Your use of underscore is correct for variables. BUT the actual rules are complex enough, that using them at the start of an identifier is not a good idea. Do you know what all the rules for underscore as the first character of an identifier are? See this SO thread for an in-depth discussion.

You use _WriteBarrier(). This identifier is reserved by the implementation. If you have written this you need to change its name. If this is provided by your implementation its non portable and you are probably using it incorrectly. Any function begin with an underscore provided by the implementation is usually private and there is a function without the underscore that provides public access to this function. Use that.

Note: The compiler is no help and will not warn you when you do it incorrectly.

Order of construction problem:

// Once again, shared_ptr might require initialization logic
// that could run after the allocation in get() [assuming we have things
// running before main]. To make this robust, use a pointer here
// and OS-provided atomic functions.
static std::shared_ptr _handle;

Don't use a pointer to solve it. It just makes the problem worse.

The best way to solve it is using a static member of a static method:

static std::shared_ptr& getHandle()
{
    static std::shared_ptr  theHandle;
    return theHandle;
}

If you are worried about construction in a multi-threaded environment. Then add a lock. Note: In gcc it is already guaranteed thread safe but you will need code for other compilers.

Your write barriers:

_frob = new singleton_t();
            _WriteBarrier();
            std::shared_ptr _derp(_frob);
            _WriteBarrier();
            _handle = move(_derp);

Lets assume your _WriteBarrier() actually works. But there is no way to determine this as there is no code for it.

Your first write barrier is a waste of time (and just superfluous code) as derp is not accessed by any other code and can only be accessed by one thread (you have caught other threads in a trap).

The second _WriteBarrier() is where you need a barrier. But it still does not do anything useful as a shared pointer is not atomic or thread safe. Thus half way through the update of this variable (_handle) one of the other threads could escape your trap above result in code being executed using a variable in a non consistent state.

Template's instantiation of members is definitely a bad idea (when done in a header file).

template
bool singleton::_bad_singleton;
template
std::atomic singleton::_spinlock;
template
std::shared_ptr singleton::_handle;

This basically creates an instance of each variable of each type that is used in every compilation unit. Though the linker will then consolidate multiple instances across compilation units for you when building a application (from simple object files) into a single variables. It will not do so correctly when building shared libraries. Because the standard has nothing to say about shared libraries. Shared libraries and the runtime linker do not have enough information to consolidate the variables across multiple shared libraries.

Thus each shared library that uses your code will have its own copy of these variables for each type that is used by the library. Thus which variable is used will depend on the library in which the code is called. Thus you can not build shared libraries using your code which makes it practically worthless.

Anyway your code is way over-complicated:
The classic singleton design is much simpler (and safer): See this.

StackExchange Code Review Q#13901, answer score: 5