tags:

views:

458

answers:

5

I have a question about the singleton pattern.

I saw two cases concerning the static member in the singleton class.

First it is an object, like this

class CMySingleton
{
public:
  static CMySingleton& Instance()
  {
    static CMySingleton singleton;
    return singleton;
  }

// Other non-static member functions
private:
  CMySingleton() {}                                  // Private constructor
  ~CMySingleton() {}
  CMySingleton(const CMySingleton&);                 // Prevent copy-construction
  CMySingleton& operator=(const CMySingleton&);      // Prevent assignment
};

One is an pointer, like this

class GlobalClass
{
    int m_value;
    static GlobalClass *s_instance;
    GlobalClass(int v = 0)
    {
        m_value = v;
    }
  public:
    int get_value()
    {
        return m_value;
    }
    void set_value(int v)
    {
        m_value = v;
    }
    static GlobalClass *instance()
    {
        if (!s_instance)
          s_instance = new GlobalClass;
        return s_instance;
    }
};

What's the difference between the two cases? Which one is correct?

+3  A: 

Neither is more correct than the other. I would tend to try to avoid the use of Singleton in general, but when I've been faced with thinking it was the way to go, I've used both of these and they worked fine.

One hitch with the pointer option is that it'll leak memory. On the other hand, your first example may end up getting destroyed before you're done with it, so you'll have a battle to wage regardless if you don't choose to figure out a more appropriate owner for this thing, that can create and destroy it at the right times.

dash-tom-bang
Leaked memory == not correct.
Billy ONeal
Ok, so? I never tried to say leaking memory was correct. Singletons are worse than memory leaks, IMO.
dash-tom-bang
@dash-tom-bang: OK. I misunderstood. Removed my downvote.
Billy ONeal
+1 for identifying how they're differently wrong - the first may be destroyed too soon, and the second won't be destroyed at all.
Mike Seymour
Thanks. To follow up, I work in an industry where our software never shuts down, indeed, it is a programming error to allow our software to terminate. So maybe I've got a different attitude than most about singly-instantiated objects "leaking," because in practice they will actually never leak.
dash-tom-bang
I've always been wondering if it leaked actually... where I work we sometimes unload the libraries and reload new versions and I wondered if the memory allocated for the singletons of the unloaded libraries remained or not since the process itself wasn't shut down...
Matthieu M.
A: 

Your first example is more typical for a singleton. Your second example differes in that it is created on-demand.

However I would try to avoid using singletons in general since they are nothing more than global variables.

Skeets
The first example is created on demand as well. Method statics are not created until their method is called for the first time.
Billy ONeal
*nothing more than global variables* ...which you sometimes need. Besides, it's wrapped up nice and neat inside its own class/namespace.
Loadmaster
+2  A: 

The difference is that the second one leaks memory (the singleton itself) while the first one does not. Static objects are initialized once the first time their associated method is called, and (so long as the program exits cleanly) they are destroyed before the program exits. The version with the pointer will leave the pointer allocated at program exit and memory checkers like Valgrind will complain.

Also, what's stopping somebody from doing delete GlobalClass::instance();?

For the above two reasons, the version using the static is the more common method and the one prescribed in the original Design Patterns book.

Billy ONeal
Do you have a citation for what's more common?
dash-tom-bang
@dash-tom-bang: Yes, the original Design Patterns book.
Billy ONeal
"more common method" I thought might reference something else, since my experience shows people using both at around even rates (and more often than not, using them wrong regardless of implementation strategy).
dash-tom-bang
Either way, the second one leaks memory, and can be misused (it returns a pointer to internals). The first one is the correct one to use.
Billy ONeal
Saying the second one leaks memory is misleading. In both cases, memory is being allocated and not released until the program terminates. In the second case, the memory is simply being taken back forcibly by the system, rather than willingly by the application. However, the destructor is never called, which is a completely different issue, which may or may not cause problems. And both provide references to internals, one just does so via a pointer. So both are equally open to abuse.
Dennis Zickefoose
@Dennis Zickefoose: That depends on the platform your program runs on. If it is a modern O/S which frees all the resources associated with a process when the process terminates, then all will be well. But if the operating system in question doesn't always clean up after processes, then resources will be leaked. This is an implementation detail however. The static method is guaranteed to be handled correctly no matter what the scenario.
Billy ONeal
dash-tom-bang
@dash-tom-bang: How can the object be accessed after destruction? It's a static variable. It's not destroyed until the program terminates.
Billy ONeal
@BillyONeal: Presuming there is more than one static object, and that they can't all be destroyed in the same instruction, it is trivial to demonstrate that a destroyed object can be accessed after creation. See the answer that starts with "You should probably read up Alexandrescu's book.", at the end of point 2.
dash-tom-bang
A: 

In response to the "memory leak" complaints, there is an easy fix:

// dtor
~GlobalClass()
{
    if (this == s_instance)
        s_instance = NULL;
}

In other words, give the class a destructor that de-initializes the hidden pointer variable when the singleton object is destructed at program termination time.

Once you've done this, the two forms are practically identical. The only significant difference is that one returns a references to a hidden object while the other returns a pointer to it.

Update

As @BillyONeal points out, this won't work because the pointed-to object never gets deleted. Ouch.

I hate to even think about it, but you could use atexit() to do the dirty work. Sheesh.

Oh, well, never mind.

Loadmaster
No, the destructor won't get called because the instance is a pointer. The destructor is not called until that pointer is `delete` d.
Billy ONeal
Aaagh, you're right. Disregard.
Loadmaster
+4  A: 

You should probably read up Alexandrescu's book.

Regarding the local static, I haven't use Visual Studio for a while, but when compiling with Visual Studio 2003, there was one local static allocated per DLL... talk about a nightmare of debugging, I'll remember that one for a while :/

1. Lifetime of a Singleton

The main issue about singletons is the lifetime management.

If you ever try to use the object, you need to be alive and kicking. The problem thus come from both the initialization and destruction, which is a common issue in C++ with globals.

The initialization is usually the easiest thing to correct. As both methods suggest, it's simple enough to initialize on first use.

The destruction is a bit more delicate. global variables are destroyed in the reverse order in which they were created. So in the local static case, you don't actually control things....

2. Local static

struct A
{
  A() { B::Instance(); C::Instance().call(); }
};

struct B
{
  ~B() { C::Instance().call(); }
  static B& Instance() { static B MI; return MI; }
};

struct C
{
  static C& Instance() { static C MI; return MI; }
  void call() {}
};

A globalA;

What's the problem here ? Let's check on the order in which the constructors and destructors are called.

First, the construction phase:

  • A globalA; is executed, A::A() is called
  • A::A() calls B::B()
  • A::A() calls C::C()

It works fine, because we initialize B and C instances on first access.

Second, the destruction phase:

  • C::~C() is called because it was the last constructed of the 3
  • B::~B() is called... oups, it attempts to access C's instance !

We thus have undefined behavior at destruction, hum...

3. The new strategy

The idea here is simple. global built-ins are initialized before the other globals, so your pointer will be set to 0 before any of the code you've written will get called, it ensures that the test:

S& S::Instance() { if (MInstance == 0) MInstance = new S(); return *MInstance; }

Will actually check whether or not the instance is correct.

However has at been said, there is a memory leak here and worst a destructor that never gets called. The solution exists, and is standardized. It is a call to the atexit function.

The atexit function let you specify an action to execute during the shutdown of the program. With that, we can write a singleton alright:

// in s.hpp
class S
{
public:
  static S& Instance(); // already defined

private:
  static void CleanUp();

  S(); // later, because that's where the work takes place
  ~S() { /* anything ? */ }

  // not copyable
  S(S const&);
  S& operator=(S const&);

  static S* MInstance;
};

// in s.cpp
S* S::MInstance = 0;

S::S() { atexit(&CleanUp); }

S::CleanUp() { delete MInstance; MInstance = 0; } // Note the = 0 bit!!!

First, let's learn more about atexit. The signature is int atexit(void (*function)(void));, ie it accepts a pointer to a function that takes nothing as argument and returns nothing either.

Second, how does it work ? Well, exactly like the previous use case: at initialization it builds up a stack of the pointers to function to call and at destruction it empties the stack one item at a time. So, in effect, the functions get called in a Last-In First-Out fashion.

What happens here then ?

  • Construction on first access (initialization is fine), I register the CleanUp method for exit time

  • Exit time: the CleanUp method gets called. It destroys the object (thus we can effectively do work in the destructor) and reset the pointer to 0 to signal it.

What happens if (like in the example with A, B and C) I call upon the instance of an already destroyed object ? Well, in this case, since I set back the pointer to 0 I'll rebuild a temporary singleton and the cycle begins anew. It won't live for long though since I am depiling my stack.

Alexandrescu called it the Phoenix Singleton as it resurrects from its ashes if it's needed after it got destroyed.

Another alternative is to have a static flag and set it to destroyed during the clean up and let the user know it didn't get an instance of the singleton, for example by returning a null pointer. The only issue I have with returning a pointer (or reference) is that you'd better hope nobody's stupid enough to call delete on it :/

4. The Monoid Pattern

Since we are talking about Singleton I think it's time to introduce the Monoid Pattern. In essence, it can be seen as a degenerated case of the Flyweight pattern, or a use of Proxy over Singleton.

The Monoid pattern is simple: all instances of the class share a common state.

I'll take the opportunity to expose the not-Phoenix implementation :)

class Monoid
{
public:
  void foo() { if (State* i = Instance()) i->foo(); }
  void bar() { if (State* i = Instance()) i->bar(); }

private:
  struct State {};

  static State* Instance();
  static void CleanUp();

  static bool MDestroyed;
  static State* MInstance;
};

// .cpp
bool Monoid::MDestroyed = false;
bool Monoid::MInstance = 0;

State* Monoid::Instance()
{
  if (!MDestroyed && MInstance)
  {
    MInstance = new State();
    atexit(&CleanUp);
  }
  return MInstance;
}

void Monoid::CleanUp()
{
  delete MInstance;
  MInstance = 0;
  MDestroyed = true;
}

What's the benefit ? It hides the fact that the state is shared, it hides the Singleton.

  • If you ever need to have 2 distinct states, it's possible that you'll manage to do it without changing every line of code that used it (replacing the Singleton by a call to a Factory for example)
  • Nodoby's going to call delete on your singleton's instance, so you really manage the state and prevent accidents... you can't do much against malicious users anyway!
  • You control the access to the singleton, so in case it's called after it's been destroyed you can handle it correctly (do nothing, log, etc...)

5. Last word

As complete as this may seem, I'd like to point out that I have happily skimmed any multithread issues... read Alexandrescu's Modern C++ to learn more!

Matthieu M.
"The only issue I have with returning a pointer (or reference) is that you'd better hope nobody's stupid enough to call delete on it :/"You've made your destructor private, so they'd have to go well out of their way to do so.
Dennis Zickefoose
Monoid -- interesting idea. I like it. It would also allow you to ref-count the internal singleton if you wanted so that it only exists if it is being used.
Skeets
@Dennis: right, forgot about that.
Matthieu M.