Writing Singleton Classes

C

cppaddict

Hi,

In this tutorial on singleton class in C++
(http://gethelp.devx.com/techtips/cpp_pro/10min/10min0200.asp) the
author gives two implementations of a simple singleton class, claiming
that only the first is safe for multi-threaded appliactions. I want
to know why this so.

The class is as follows:

class Singleton
{
public:
static Singleton* Instance();
protected:
Singleton();
Singleton(const Singleton&);
Singleton& operator= (const Singleton&);
private:
static Singleton* pinstance;
};

The first implementation is:

Singleton* Singleton::pinstance = 0;// initialize pointer
Singleton* Singleton::Instance ()
{
if (pinstance == 0) // is it the first call?
{
pinstance = new Singleton; // create sole instance
}
return pinstance; // address of sole instance
}
Singleton::Singleton()
{
//... perform necessary instance initializations
}

The second implementation makes a small change to the constructor:

Singleton* Singleton::Instance ()
{
static Singleton inst;
return &inst;
}

The author notes that the second implementation is optimized for
single-threaded applications. Why would it not also work for
multi-threaded applications?

Thanks,
cpp
 
R

red floyd

cppaddict said:
Hi,

In this tutorial on singleton class in C++
(http://gethelp.devx.com/techtips/cpp_pro/10min/10min0200.asp) the
author gives two implementations of a simple singleton class, claiming
that only the first is safe for multi-threaded appliactions. I want
to know why this so.

The class is as follows:

class Singleton
{
public:
static Singleton* Instance();
protected:
Singleton();
Singleton(const Singleton&);
Singleton& operator= (const Singleton&);
private:
static Singleton* pinstance;
};

Looks to me like the *first* instance isn't thread safe, but the second is!
The first implementation is:

Singleton* Singleton::pinstance = 0;// initialize pointer
Singleton* Singleton::Instance ()
{
if (pinstance == 0) // is it the first call?
{
What happens if a context switch occurs right here? After the if
statemnt, but before the new?
pinstance = new Singleton; // create sole instance
}
return pinstance; // address of sole instance
}
Singleton::Singleton()
{
//... perform necessary instance initializations
}

The second implementation makes a small change to the constructor:
Not a constructor, but the Singleton accessor.As far as I can tell, this *is* thread safe.
Singleton* Singleton::Instance ()
{
static Singleton inst;
return &inst;
}

The author notes that the second implementation is optimized for
single-threaded applications. Why would it not also work for
multi-threaded applications?

I think the author is full of it, but that's just *MY* opinion... I
could be wrong.
 
B

Benoit Mathieu

cppaddict said:
Hi,

In this tutorial on singleton class in C++
(http://gethelp.devx.com/techtips/cpp_pro/10min/10min0200.asp) the
author gives two implementations of a simple singleton class, claiming
that only the first is safe for multi-threaded appliactions. I want
to know why this so.
...

I found an answer here :
http://blogs.msdn.com/oldnewthing/archive/2004/03/08/85901.aspx

Now, the first implementation is also problematic, and has
exactely the same problem: initialisation must be protected
by a mutex or something...

Benoit
 
C

cppaddict

Now, the first implementation is also problematic, and has
exactely the same problem: initialisation must be protected
by a mutex or something...

Benoit

Thanks for the reference. That's very interesting....

What is a mutex though? After reading that article it seems that
Singletons are never thread safe.

Thanks,
cpp
 
T

Tim Clacy

cppaddict said:
Hi,

In this tutorial on singleton class in C++
(http://gethelp.devx.com/techtips/cpp_pro/10min/10min0200.asp) the
author gives two implementations of a simple singleton class, claiming
that only the first is safe for multi-threaded appliactions. I want
to know why this so.

cppaddict,

The first implementation needs a slight change to be thread-safe:

Singleton* Singleton::Instance ()
{
// In the common case, simply return the existing instance (no
locking required)
//
if (pinstance != 0)
return pinstance;

// If there was no instance above, we need to lock now and
double-check whether
// another thread created an instance in between the check above and
now
//
Lock();
if (pinstance == 0)
pinstance = new Singleton;
Unlock();

return pinstance;
}

There are a dozen or so Singleton articles on CodeProject; the following is
very good and covers threading issues:

Singleton Pattern: A review and analysis of existing C++ implementations
http://www.codeproject.com/cpp/singleton.asp


Tim
 
B

Benoit Mathieu

What is a mutex though? After reading that article it seems that
Singletons are never thread safe.

Mutex means Mutual Exclusion (it's a mechanism that ensures
that at most one thread accesses a particular object at a
given time). See Tim Clacy's post: Lock() prevents other
threads from executing the sequence until Unlock() has been
called. Lock and Unlock can usually not be implemented with
high level standard instructions. It always involves some
machine specific instructions (disabling interruptions, ...)
but this is going to be very off-topic here and you will
have to switch to another group to get informations about
locking mechanisms.

Local static variables are not safe, but implementing a
Singleton with Tim Clacy's code is safe (provided that
Lock() and Unlock() are correct, of course) :

Lock();
if (pinstance == 0)
pinstance = new Singleton;
Unlock();

Benoit
 
X

Xenos

red floyd said:
As far as I can tell, this *is* thread safe.
No, its not. If the first thread is in the middle of construction, and get
preempted, the object is in an unknown state when the second thread attempts
access. This is safe from order dependencies during init., but not thread
safe. The opposite is true for the first form. It is thread-safe because
it is initialized before the application's main procedure runs and therefore
before the program can spawn any threads. It is not safe from order
dependencies if a staticly constructed object from a different file attempts
access to it during construction.
 
X

Xenos

Xenos said:
No, its not. If the first thread is in the middle of construction, and get
preempted, the object is in an unknown state when the second thread attempts
access. This is safe from order dependencies during init., but not thread
safe. The opposite is true for the first form. It is thread-safe because
it is initialized before the application's main procedure runs and therefore
before the program can spawn any threads. It is not safe from order
dependencies if a staticly constructed object from a different file attempts
access to it during construction.

Looked at code to quick, the first is not thread-safe either. I thought it
was constructor at startup, not is not.
 
X

Xenos

Xenos said:
Looked at code to quick, the first is not thread-safe either. I thought it
was constructor at startup, not is not.

Geesh! Now I'm just typing to fast.

....constructed at strartup, but it is not!
 
D

DaKoadMunky

Neither of the solutions in the article originally referenced are thread safe.

In a threaded program both could lead to undesired behavior such as double
construction or premature access.

One of the respondents to this thread posted an example of the Double Checked
Locking idiom.

Apparently even that is problematic as discussed by Scott Meyers in the
document @ http://www.nwcpp.org/Downloads/2004/DCLP_notes.pdf
 
A

Alexander Terekhov

DaKoadMunky wrote:
[...]
One of the respondents to this thread posted an example of the Double Checked
Locking idiom.

The idiom has only one check for locking, not two. I guess that
someone had called it "the DCL" after 10th drink or so (they say
that after about 10th drink one tends not to count correctly
anymore). A much better name is "Double Checked Serialized
Initialization". There are two variations of DCSI. One with
atomic<>-with-msync::<stuff> and more portable one with thread-
specific data (e.g. "thread_specific_ptr<>") instead of
atomic<>-with-msync::<stuff> (and I don't mean thread-specific
singletons). There is also "Double Checked Concurrent
Initialization" idiom (lock-less and not for "singletons").

http://google.com/[email protected]
Apparently even that is problematic as discussed by Scott Meyers in the
document @ http://www.nwcpp.org/Downloads/2004/DCLP_notes.pdf

Not bad up to Pg. 22. "Multiprocessors, Cache Coherency, and
Memory Barriers" part is somewhat screwed. For example, Pg. 34
and Pg. 35:

Keyboard* temp = pInstance;
Perform acquire;
...

(that notation sucks, BTW) is not really the same (with
respect to reordering) as

Keyboard* temp = pInstance;
Lock L1(args); // acquire
...

because the later can be transformed to

Lock L1(args); // acquire
Keyboard* temp = pInstance;
...

While it does stress the point of Pg. 36, the difference is
quite significant and it can really hurt you in some other
context. Beware.

regards,
alexander.

P.S. atomic<> is the way to go.
 
J

Joe Seigh

Alexander said:
DaKoadMunky wrote:
[...]
One of the respondents to this thread posted an example of the Double Checked
Locking idiom.

The idiom has only one check for locking, not two. I guess that
someone had called it "the DCL" after 10th drink or so (they say
that after about 10th drink one tends not to count correctly
anymore). A much better name is "Double Checked Serialized
Initialization". There are two variations of DCSI. One with
atomic<>-with-msync::<stuff> and more portable one with thread-
specific data (e.g. "thread_specific_ptr<>") instead of
atomic<>-with-msync::<stuff> (and I don't mean thread-specific
singletons). There is also "Double Checked Concurrent
Initialization" idiom (lock-less and not for "singletons").

http://google.com/[email protected]

Or you would just access the singleton via a reference in a handle
or object where correct visibility was established in the handle
or objects ctor, said objects being handled in the usual "threadsafe
as int" idiom.

....
P.S. atomic<> is the way to go.

except that there are several possibilities for atomic<> semantics
besides just the mutable/immutable stuff. There's atomic<T> vs.
atomic<*T>.

Joe Seigh
 
A

Alexander Terekhov

Joe Seigh wrote:
[...]
Or you would just access the singleton via a reference in a handle
or object where correct visibility was established in the handle
or objects ctor,

Yep. For example auto_unlock_ptr<>. That's what you need for mutable
"singletons" that don't use atomic<> internally. They need a lock
anyway and, usually, there's no problem to initialize the lock eagerly
or use something like lazy create-open-named-mutex trick on windows.
The situation is a bit different for immutable things (but you can
still use "expensive" get_instance() and cache the reference on the
callers side; in a way, that's what the TSD variation of DCSI does
internally).

regards,
alexander.
 
J

Joe Seigh

Alexander said:
Joe Seigh wrote:
[...]
Or you would just access the singleton via a reference in a handle
or object where correct visibility was established in the handle
or objects ctor,

Yep. For example auto_unlock_ptr<>. That's what you need for mutable
"singletons" that don't use atomic<> internally. They need a lock
anyway and, usually, there's no problem to initialize the lock eagerly
or use something like lazy create-open-named-mutex trick on windows.
The situation is a bit different for immutable things (but you can
still use "expensive" get_instance() and cache the reference on the
callers side; in a way, that's what the TSD variation of DCSI does
internally).

I mean caching the reference in the object. In the object's ctor
you use DCL or one of your acronyms to create the singleton and
store the reference to the singleton in the object. The "safe
as int" visibility semantics guarantee proper visibility to the
singleton the same way they guarantee proper visibility of any
of the object's fields. It's not specific or local to the thread,
it's specific to the object using proper ownership rules.

If you are only accessing the singleton via the object's methods,
then it's pretty much transparent to the user. The only way
the user could get into trouble is by violating the "safe as int"
rule and they'd be in trouble even without a singleton involved.

Joe Seigh
 
M

Mark A. Gibbs

DaKoadMunky wrote:

One of the respondents to this thread posted an example of the Double Checked
Locking idiom.

Apparently even that is problematic as discussed by Scott Meyers in the
document @ http://www.nwcpp.org/Downloads/2004/DCLP_notes.pdf

am i missing something here? why wouldn't this work:

Singleton* Singleton::Instance()
{
if (pinstance != 0)
return pinstance;

Lock();

if (pinstance == 0)
pinstance = makeSingleton();

Unlock();

return pinstance;
}

Singleton* Singleton::makeSingleton()
{
return new Singleton;
}

mark
 
R

red floyd

Mark said:
DaKoadMunky wrote:




am i missing something here? why wouldn't this work:

Singleton* Singleton::Instance()
{
if (pinstance != 0)
return pinstance;

Lock();

if (pinstance == 0)
pinstance = makeSingleton();

Unlock();

return pinstance;
}

Singleton* Singleton::makeSingleton()
{
return new Singleton;
}

mark

Andrei Alexandrescu discusses this in "Modern C++ Design". It has to do
with memory architectures. In some cases, you need to do hardware
specific stuff to ensure cache coherency. He also recommends making
pinstance volatile.
 
M

Mark A. Gibbs

red said:
Andrei Alexandrescu discusses this in "Modern C++ Design". It has to do
with memory architectures. In some cases, you need to do hardware
specific stuff to ensure cache coherency. He also recommends making
pinstance volatile.

I had to dig up my copy to check what you meant, and the only concern I
could see addressed is based on architectures that queue up memory
writes. Is that what you meant?

In a case like this, couldn't you rewrite:

Singleton* Singleton::makeSingleton()
{
Singleton* temp = 0;

try
{
ImplementationSpecificLock();
Singleton* temp = new Singleton;
ImplementationSpecificUnlock();
}
catch(...)
{
ImplementationSpecificUnlock();
throw;
}

return temp;
}

for those implementations that require it?

On a side note, how common are such architectures?

Andrei Alexandrescu doesn't actually describe the type of singleton I
use most often, which is kind of a variation on the phoenix singleton.
I'm not sure if the pattern I use has a name, but it requires explicit
lifetime control, allows for optional construction (lazy instantiation)
and external locking and checking.

template <typename T>
class Singleton
{
public:
typedef implementation_defined Mutex;

static Mutex& GetMutex() { return mutex_; }

static bool InstanceExists() { return !(instance_ = 0); }

static T& Instance()
{
if (!InstanceExists()) throw SingletonDoesNotExistException;
return *instance_;
}

protected:
explicit Singleton(T* t = 0)
{
Mutex::Lock lock(GetMutex());

if (instance_) throw MultipleSingletonException;

instance_ = t ? t : static_cast<T*>(this);
}

~Singleton()
{
Mutex::Lock lock(GetMutex());

instance_ = 0;
}

private:
static Mutex mutex_;
static T* instance_;
};

And used like this:

class Foo : public Singleton<Foo>
{
public:
void func1();
void func2();
};

The thinking I've always used behind this design is that I'd like to be
responsible for when the mutex is locked and unlocked, although
individual functions are free to lock it just in case (multiple locks
within a thread are ok, but each lock must be matched by an unlock). I'd
also like to be able to control when and how the singleton is created,
and potentially replace it on the fly in some cases.

For example:

// In this case, I use Foo if it is available
Foo::Mutex::Lock lock(Foo::GetMutex());
if (Foo::InstanceExists()) { /*use it*/ }

// I can also call multiple functions without relocking
Foo::Mutex::Lock lock(Foo::GetMutex());
Foo& foo = Foo::Instance();
foo.func1();
foo.func2();

// And I can swap objects on the fly
class Base : public Singleton<Base>
{
public:
virtual ~Base();

virtual void func1() = 0;
virtual void func2() = 0;

protected:
Base();
};

class Derived1 : public Base
{
public:
void func1();
void func2();
};

class Derived2 : public Base
{
public:
void func1();
void func2();
};

Base::Mutex::Lock lock(Base::GetMutex());
Base* base = &Base::Instance(); // Currently a Derived1
delete base;
base = new Derived2;
Base& new_base = &Base::Instance();
new_base.func1();
new_base.func2();

This doesn't seem to suffer from any instruction ordering problem to me,
and the cached-memory-write architecture problem could be handled in the
mutex class. How does this measure up?

mark
 
Joined
Sep 14, 2008
Messages
1
Reaction score
0
Tim Clacy said:
cppaddict wrote:

Singleton* Singleton::Instance ()
{
// In the common case, simply return the existing instance (no
locking required)
//
if (pinstance != 0)
return pinstance;

// If there was no instance above, we need to lock now and
double-check whether
// another thread created an instance in between the check above and
now
//
Lock();
if (pinstance == 0)
pinstance = new Singleton;
Unlock();

return pinstance;
}

Tim


For this to be thread-safe you need to assume that the assignment will be made with a single instruction. I know it actually will. But in my opinion it's much better to write programs in a way that doesn't assume anything. Maybe it would be better to write:

if (initialized)
return pinstance;

Lock ();
if (!initialized)
pinstance = new Singleton;
initialized = 1;
Unlock ();

return pinstance;

Now the condition cannot be true before pinstance is properly initialized.
 

Ask a Question

Want to reply to this thread or ask your own question?

You'll need to choose a username for the site, which only take a couple of moments. After that, you can post your question and our members will help you out.

Ask a Question

Members online

Forum statistics

Threads
473,967
Messages
2,570,148
Members
46,694
Latest member
LetaCadwal

Latest Threads

Top