What is the correct way to check if a singleton instance has been instantiated or not?

Question

I have seen the following two examples:

class singleton {
    protected:
        static singleton *instance;
        singleton() { }
    public:
        static singleton *getInstance() {
            if (instance == 0)
                instance = new singleton();
            return instance;
        }
};
singleton *singleton::instance = 0; //This seems weird - why isn't nullptr being used ?

And this example:

class Singleton
{
  private:

    static Singleton *p_inst;
    Singleton();

  public:

    static Singleton * instance()
    {
      if (!p_inst) // why isn't this being compared with nullptr ?
      {
        p_inst = new Singleton();
      }

      return p_inst;
    }
};

To check if the instance has been created, why don't people do something like this:

class Singleton
{
  private:

    static Singleton *p_inst = nullptr;
    Singleton();

  public:

    static Singleton * instance()
    {
      if (p_inst != nullptr) // why isn't this being compared with nullptr ?
      {
        p_inst = new Singleton();
      }

      return p_inst;
    }
};

What is the correct way ?

EDIT: After all the responses below, I'm really confused. What is the definitive way to create a thread-safe singleton, without any bugs.

Answer 1

Note that none of the examples you have listed actually attempts to delete the singleton, and none of them are thread-safe, because they do nothing to synchronise their:

• compare to null (of various flavours)
• their allocation of the object
• their assignment of the pointer

None of your existing methods are able to safely test if the singleton has yet been created.

C++11 adds a guarantee for the method's static data member that its initialisation will be thread-safe - i.e. only called once. Tghis is the recommended mechanism. The classic singleton is something like:

SClass& Singleton()
{ 
  static SClass single(args);
  return single;
}

If you really want the object to be heap allocated:

SClass* Singleton()
{ 
  static std::unique_ptr<SClass> single (new SClass(args));
  return single;
}

[ Note that you can use function calls and/or lambdas in the construction, if your object construction isn't a simple new. You can write lambdas as the second argument of the std::unique_ptr to get complex destruction, so you can easily customise within this model. ]

If you really want to do it without using a static member, you could use a std::mutex or a std::atomic with cmpexchange, but in c++11 the static member does that work for you.

Also the object will be destroyed, and in reverse order to the order they were created. However, that can lead to phoenix issues, where SomeThing calls on the singleton during it's destruction, and it was constructed earlier than your singleton, so gets destructed later.

The versions you are looking at simply never delete the object, which will make valgrind very unhappy. If you want your existing behaviour, with a thread-safe construction, but no destruction, use a raw pointer (they don't have destructors):

SClass* Singleton()
{ 
  static SClass* single = new SClass(args);
  return single;
}

The retroactive fix is to have that SomeThing call on the singleton during its construction, or be able to handle a nullptr on return. Of course you have to debug that scenario before you can fix it. So I recommend you at least add an abend to your singleton so that you know why it is failing: [Actually, as flagged in the comments this is not so trivial, as unique_ptr might not leave itself in a good state after destruction, so we need a custom ptr lifetime handler. Luckily, we don't really NEED a full-blown unique_ptr here, just 3 or 4 methods. Potentially this walks into UB, as the compiler could optimise away this null assignment during destruction, or DEADBEEF this location, but phoenixes should not be relied on in production code anyway.]

SClass* Singleton()
{ 
  struct PUP: std::unique_ptr<SClass> single
  {
    typedef std::unique_ptr<SClass> base;
    PUP(SClass* s): base(s) {}
    ~PUP()  { operator =( base()_); }
  };
  static PUP single (new SClass(args));
  assert(single && "Singleton has been called in late static destructor - need phoenix");
  return single;
}

You can, if you like, have the object rise from the dead. This should be safe to do as static destruction is single-threaded. Note that the re-arissen object can never be destroyed, and does not keep any of the state of the old object. This can be useful for logging classes, where someone adds logging in a destructor not realising the logging class singleton has already been destroyed. For a diagnostic, this is useful. For production code, it will still upset valgrind, just don't do it!

SClass* Singleton()
{ 
  static PUP single = new SClass(args);
  if (!single)
  {
    single = std::unique_ptr<SClass>(new SClass(args));
    // Hmm, and here is another phoenix being revived:
    Logger(CRITICAL) << " Revived singleton for class SClass!" << std::endl;
  }
  return single;
}