46

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?

Improve this question
2
  • 15
    If you're really interested, Alexandrescu's "Modern C++ Design" has a whole chapter devoted to trying to make singletons safe and correct, exploring many dark corners of the language. In my opinion, it can be summarised as "just don't". Commented Mar 23, 2010 at 1:43
  • @Mike - great reference, and I agree wholly. Commented Mar 23, 2010 at 1:45

8 Answers 8

Reset to default
61

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; State* 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!

Improve this answer
Sign up to request clarification or add additional context in comments.

4 Comments

"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.
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.
Very interesting, thanks a lot :) I think there are two little typos: State* Monoid::MInstance = 0; and if (!MDestroyed && !MInstance)
@Sh3ljohn: and you are right, and I had missed your post until then, and somehow sid's edit to patch those was rejected...
4

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.

Improve this answer

4 Comments

Ok, so? I never tried to say leaking memory was correct. Singletons are worse than memory leaks, IMO.
+1 for identifying how they're differently wrong - the first may be destroyed too soon, and the second won't be destroyed at all.
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.
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...
2

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.

Improve this answer

9 Comments

Do you have a citation for what's more common?
@dash-tom-bang: Yes, the original Design Patterns book.
"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).
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.
@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.
|
1

Use second approach - if you don't want to use atexit to free your object, then you can always use keeper object (eg. auto_ptr, or something self written). This might cause freeing before you are done with object, just as with first first method.

The difference is that if you use static object, you basically have no way to check if it already got freed or not.

If you use pointer, you can add additional static bool to indicate if singleton got already destroyed (as in Monoid). Then your code can always check if singleton was already destroyed, and although you might fail at what you intend to do, at least you will not get cryptic "segmentaion fault" or "access violation", and the program will avoid abnormal termination.

Improve this answer

Comments

1

I agree with Billy. In 2nd approach we are dynamically allocating memory from the heap using new. This memory remains always and never gets freed, unless a call to delete is been made. Hence the Global pointer approach creates a memory leak.

class singleton { private: static singleton* single; singleton() { } singleton(const singleton& obj) { } public: static singleton* getInstance(); ~singleton() { if(single != NULL) { single = NULL; } } }; singleton* singleton :: single=NULL; singleton* singleton :: getInstance() { if(single == NULL) { single = new singleton; } return single; } int main() { singleton *ptrobj = singleton::getInstance(); delete ptrobj; singleton::getInstance(); delete singleton::getInstance(); return 0; }
Improve this answer

Comments

0

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.

Improve this answer

2 Comments

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

A better approach is to create a singleton class. This also avoids the instance availability check in the GetInstance() function. This can be achieved using a function pointer.

class TSingleton; typedef TSingleton* (*FuncPtr) (void); class TSingleton { TSingleton(); //prevent public object creation TSingleton (const TSingleton& pObject); // prevent copying object static TSingleton* vObject; // single object of a class static TSingleton* CreateInstance (void); static TSingleton* Instance (void); public: static FuncPtr GetInstance; }; FuncPtr TSingleton::GetInstance = CreateInstance; TSingleton* TSingleton::vObject; TSingleton::TSingleton() { } TSingleton::TSingleton(const TSingleton& pObject) { } TSingleton* TSingleton::CreateInstance(void) { if(vObject == NULL){ // Introduce here some code for taking lock for thread safe creation //... //... //... if(vObject == NULL){ vObject = new TSingleton(); GetInstance = Instance; } } return vObject; } TSingleton* TSingleton::Instance(void) { return vObject; } void main() { TSingleton::GetInstance(); // this will call TSingleton::Createinstance() TSingleton::GetInstance(); // this will call TSingleton::Instance() // all further calls to TSingleton::GetInstance will call TSingleton::Instance() which simply returns already created object. }
Improve this answer

Comments

-1

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.

Improve this answer

1 Comment

No, the destructor won't get called because the instance is a pointer. The destructor is not called until that pointer is delete d.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.