Confused about a thread-safe singleton example.

J

jason.cipriani

I have a C++-specific question about thread-safe singleton instances.
There's a trivial example here:

http://www.bombaydigital.com/arenared/2005/10/25/1

That goes like this, very simple and straightforward:

static Mutex mutex;
static TheClass *instance;

static TheClass * getInstance () {
MutexLocker lock(mutex);
if (!instance)
instance = new TheClass();
return instance;
}

The example then goes on to talk about how double-check locking is
broken, etc. My question is pretty much this: Is C++ static
initialization thread-safe? If not, then how does the above example
safely use "mutex"? If so, then what is wrong with this:

static TheClass instance; // not a pointer

static TheClass * getInstance () {
return &instance; // it's correctly initialized?
}

The reason I ask is I almost never see it done like that, I always see
blog entries and articles that say the same thing "store instance in a
pointer, use a mutex to protect, and p.s. double-checked locking is
broken". It seems like doing it lock-free is made out to be a hard
problem, so *if* having a static instance works (but I don't know if
it does, that's my question), then why doesn't anybody ever suggest
it?

Thanks!
Jason
 
J

jason.cipriani

Setting aside the fact that there's no such thing as threads or mutexes in
the C++ language (at least not yet), so you are using a platform specific
library here.

I just used "Mutex" and "AutoMutex" as an example.
Your statically declared instance gets constructed at some unspecified point
before your main() function gets invoked. If you have other objects in
static scope, it is unspecified the order in which all your static instances
get initialized. This may be undesirable. It's possible that it is necessary
to construct your singleton in a more controlled fashion, after all your
other objects, in static scope or otherwise, get initialized. Using a
dynamically-allocated pointer to your singleton, and protecting it with a
mutex, gives you the means to accomplish that.

I see. So, it's safe to use a global-scoped static instance for the
singleton instance, as long as you don't need *precise* control over
when it's initialized (just as long as it's initialized before it's
used)? Even if it's accessed from different translation units (and
defined in a different one than main() is in)?

I did an experiment with VS 2008 where I made the singleton class's
constructor Sleep() for 2 seconds hoping to make a race condition
occur, and did this:

=== A.h ===
class A {
public:
A ();
~A ();
static A * getInstance ();
};

=== A.cpp ===
static A theInstance;
A * A::getInstance () { return &theInstance; }

I had main() in a different source file, and it created some threads
with functions in a 3rd source file. I called A::getInstance() in each
of those threads, and saw that theInstance was initialized before main
() was even entered, and everything worked great.

Is this standard behavior that I can rely on, or is it specific to the
MS compiler?

I also tried making theInstance function-scoped, in the getInstance()
function. That didn't work, I guess there's different rules for
function-scoped static initialization (I did read that, and also read C
++0x makes some guarantees about it). I noticed that if I created
multiple threads like this:

threadproc () {
A * a = A::getInstance();
}

The first thread created waited the 2 seconds as the A() was
constructed, but every thread after that immediately returned, *with*
the pointer, but before the A() constructor had returned.

A * A::getInstance () {
static A theInstance;
return &theInstance;
}

I guess that makes sense. "&theInstance" is already known, so threads
can return immediately while it's still being constructed.

The third thing I tried was storing theInstance at class scope, and
using a pointer but statically initializing it with new(). That also
worked, it was initialized before main() was entered. Is this also
behavior that can be relied on? E.g.:

class A {
static A * getInstance ();
...
static A * theInstance;
};

A * A::theInstance = new A();

A * A::getInstance () { return theInstance; }


So, global scope worked, class scope worked, function scope was all
messed up.


Thanks,
Jason
 
A

Alan Johnson

I have a C++-specific question about thread-safe singleton instances.
There's a trivial example here:

http://www.bombaydigital.com/arenared/2005/10/25/1

That goes like this, very simple and straightforward:

static Mutex mutex; static TheClass *instance;

static TheClass * getInstance () { MutexLocker lock(mutex); if
(!instance) instance = new TheClass(); return instance; }

The example then goes on to talk about how double-check locking is
broken, etc. My question is pretty much this: Is C++ static
initialization thread-safe? If not, then how does the above example
safely use "mutex"? If so, then what is wrong with this:

static TheClass instance; // not a pointer

static TheClass * getInstance () { return &instance; // it's
correctly initialized? }

The reason I ask is I almost never see it done like that, I always
see blog entries and articles that say the same thing "store instance
in a pointer, use a mutex to protect, and p.s. double-checked locking
is broken". It seems like doing it lock-free is made out to be a hard
problem, so *if* having a static instance works (but I don't know if
it does, that's my question), then why doesn't anybody ever suggest
it?

Thanks! Jason

1. Don't use singletons. Ever. Pretty much all of the value of the GoF
Design Patterns book is negated by the fact that they chose to
legitimize Singleton as a design pattern. Singleton is just a fancy
name for global variable. We should call it the Global Variable
(anti)Pattern.

2. If you think you've found a good reason to use a singleton, see point #1.

3. Double checked locking works without memory fencing on x86:
http://bartoszmilewski.wordpress.com/2008/11/05/who-ordered-memory-fences-on-an-x86/

Thus, your main opponent in the optimizer.

4. Whether or not the construction of a static variable is threadsafe
depends mostly on the implementation (the standard says nothing about
it). gcc has an option =fno-threadsafe-statics to turn off the extra
code emitted to make local statics thread safe. I guess one could
extrapolate that by default local statics are thread safe in gcc (though
I have no idea if this is actually true).
 
A

anon

I have a C++-specific question about thread-safe singleton instances.
There's a trivial example here:

http://www.bombaydigital.com/arenared/2005/10/25/1

That goes like this, very simple and straightforward:

static Mutex mutex;
static TheClass *instance;

static TheClass * getInstance () {
MutexLocker lock(mutex);
if (!instance)
instance = new TheClass();
return instance;
}

Where do you unlock the mutex?
The example then goes on to talk about how double-check locking is
broken, etc. My question is pretty much this: Is C++ static
initialization thread-safe? If not, then how does the above example
safely use "mutex"? If so, then what is wrong with this:

static TheClass instance; // not a pointer

static TheClass * getInstance () {
return &instance; // it's correctly initialized?
}

You do not have a compilable code, but if instance is global variable,
then it is initialized before entering main(). Therefore, before any
thread is created

btw in this case, the getInstance function can be like this:
static TheClass & getInstance () {
return instance; // it's correctly initialized?
}
The reason I ask is I almost never see it done like that, I always see
blog entries and articles that say the same thing "store instance in a
pointer, use a mutex to protect, and p.s. double-checked locking is
broken". It seems like doing it lock-free is made out to be a hard
problem, so *if* having a static instance works (but I don't know if
it does, that's my question), then why doesn't anybody ever suggest
it?

Thanks!
Jason


I think your suggestion should be like this:

static TheClass * getInstance () {
MutexLocker lock(mutex);
return &instance; // it's correctly initialized?
}
because in the first function, there is a mutex locked, so only one
thread can access the class instance at the time



Sorry I didn't answer your question - I do not know the answer :(
 
J

jason.cipriani

1. Don't use singletons.  Ever.  Pretty much all of the value of the GoF
Design Patterns book is negated by the fact that they chose to
legitimize Singleton as a design pattern.  Singleton is just a fancy
name for global variable.  We should call it the Global Variable
(anti)Pattern.

2. If you think you've found a good reason to use a singleton, see point #1.

In this case, the object in question is used as a "default object"
when some other non-default object isn't explicitly used (sorry I'm
being vague, but there's not much need to explain it). Maintaining a
single instance of the "default object" eliminates the need to have to
clean up separate default object instances, and to keep track of
whether or not clean up is necessary, who owns the object, etc. It
greatly simplifies a lot of application code. And you are correct, in
this case it basically is, in fact, a global variable.
3. Double checked locking works without memory fencing on x86:http://bartoszmilewski.wordpress.com/2008/11/05/who-ordered-memory-fe...

Thus, your main opponent in the optimizer.

4. Whether or not the construction of a static variable is threadsafe
depends mostly on the implementation (the standard says nothing about
it). gcc has an option =fno-threadsafe-statics to turn off the extra
code emitted to make local statics thread safe.  I guess one could
extrapolate that by default local statics are thread safe in gcc (though
I have no idea if this is actually true).

I think you may be talking about function-local statics. C++
guarantees that global-scoped statics are initialized before main().
This means that, as long as the singleton class initialization doesn't
depend on objects being initialized in other translation units (and
the other way around), it *will* be properly initialized before main
(), and therefore it will be properly initialized before any threads
access it (assuming all threads are created after main starts, not
during static initialization). So no locking would be needed (on any
platforms); getInstance() would always return a completely initialized
object -- here is the method I finally settled on:

=== MyClass.h ===

class MyClass {
....
static MyClass * getInstance ();
....
};

=== MyClass.cpp ===

static MyClass *instance = new MyClass();

MyClass * MyClass::getInstance () {
return instance;
}

The same appears to be true for class-scoped statics as well, but I
didn't do any more digging because the above solved the problem.

Thanks,
Jason
 
A

anon

On Dec 3, 1:47 am, Alan Johnson <[email protected]> wrote:

[lots of cutting]
greatly simplifies a lot of application code. And you are correct, in
this case it basically is, in fact, a global variable.

Both of you are not correct. The singleton is not a global variable.

TheClass* instance = NULL;
TheClass * GetInstance()
{
if ( NULL == instance )
{
instance = new TheClass;
}
return instance;
}

In this case, the instance is created when the function GetInstance()
gets called the first time, while global variables gets created before
entering main()
 
J

James Kanze

1. Don't use singletons. Ever. Pretty much all of the value
of the GoF Design Patterns book is negated by the fact that
they chose to legitimize Singleton as a design pattern.
Singleton is just a fancy name for global variable. We should
call it the Global Variable (anti)Pattern.

And there are valid reasons for using it. Some things must be
inherently unique in a process.
3. Double checked locking works without memory fencing on
x86:http://bartoszmilewski.wordpress.com/2008/11/05/who-ordered-memory-fe...

The author says that it works without fences, but he doesn't
give any reference to anything which guarantees it. (From a
quick glance at the AMD document, it looks like it might be
safe, but I'd need more analysis to be sure.)
Thus, your main opponent in the optimizer.
4. Whether or not the construction of a static variable is
threadsafe depends mostly on the implementation (the standard
says nothing about it). gcc has an option
=fno-threadsafe-statics to turn off the extra code emitted to
make local statics thread safe. I guess one could extrapolate
that by default local statics are thread safe in gcc (though I
have no idea if this is actually true).

Not that the g++ implementation of this is broken on some
processors.
 
J

James Kanze

I have a C++-specific question about thread-safe singleton instances.
There's a trivial example here:

That goes like this, very simple and straightforward:
static Mutex mutex;
static TheClass *instance;
static TheClass * getInstance () {
MutexLocker lock(mutex);
if (!instance)
instance = new TheClass();
return instance;
}
The example then goes on to talk about how double-check
locking is broken, etc. My question is pretty much this: Is
C++ static initialization thread-safe?

No.

It's actually very hard to answer concretely here. Objects with
static lifetime in class or namespace scope are constructed
before main is entered, so if threads aren't started until then,
you're OK. Except that if getInstance is called before then,
you may have an order of initialization problem. And the
guarantee of construction before main is entered doesn't apply
to dynamically linked objects (which aren't addressed by the
standard, but then, neither is multi-threading).

My usual solution here is something like:

namespace {
TheClass* ourInstance = &TheClass::instance() ;
}

TheClass&
TheClass::instance()
{
if ( ourInstance == NULL ) {
ourInstance = new TheClass ;
// or
static TheClass theOneAndOnly ;
ourInstance = &theOneAndOnly ;
}
return *ourInstance ;
}

The initializer for the static variable ensures that
TheClass::instance is called at least once before main is
entered, and once you've returned from a call to
TheClass::instance(), the function is thread safe without locks.
If not, then how does the above example safely use "mutex"? If
so, then what is wrong with this:
static TheClass instance; // not a pointer
static TheClass * getInstance () {
return &instance; // it's correctly initialized?
}
The reason I ask is I almost never see it done like that, I
always see blog entries and articles that say the same thing
"store instance in a pointer, use a mutex to protect, and p.s.
double-checked locking is broken". It seems like doing it
lock-free is made out to be a hard problem, so *if* having a
static instance works (but I don't know if it does, that's my
question), then why doesn't anybody ever suggest it?

Because it's open to order of initialization issues.
 
A

Alan Johnson

anon said:
On Dec 3, 1:47 am, Alan Johnson <[email protected]> wrote:

[lots of cutting]
greatly simplifies a lot of application code. And you are correct,
in this case it basically is, in fact, a global variable.

Both of you are not correct. The singleton is not a global variable.

TheClass* instance = NULL; TheClass * GetInstance() { if ( NULL ==
instance ) { instance = new TheClass; } return instance; }

In this case, the instance is created when the function GetInstance()
gets called the first time, while global variables gets created
before entering main()

That still has all the negative effects of a global variable. To name a few
= hidden dependencies in functions that use it
= persistent global state
- tight coupling between anything that uses it

It also brings its own set of new problems.
= multiple responsibilities in one class (responsible for limiting the
number of instances and for whatever the class is normally supposed to do)
= the whole cluster of problems around thread-safe lazy initialization
- someday you will want more than one (you always do)
- typically unable to derive from the singleton class (without heroic
effort)

You would typically be better off just declaring a global
variable and using that instead.
 
A

Alan Johnson

I think you may be talking about function-local statics. C++
guarantees that global-scoped statics are initialized before main().
This means that, as long as the singleton class initialization
doesn't depend on objects being initialized in other translation
units (and the other way around), it *will* be properly initialized
before main (), and therefore it will be properly initialized before
any threads access it (assuming all threads are created after main
starts, not during static initialization). So no locking would be
needed (on any platforms); getInstance() would always return a
completely initialized object -- here is the method I finally settled
on:

If you are going to declare it as a global variable, then why go through
the trouble of hiding it behind a function and calling it a singleton?
 
M

Maxim Yegorushkin

1. Don't use singletons.  Ever.  Pretty much all of the value of the GoF
Design Patterns book is negated by the fact that they chose to
legitimize Singleton as a design pattern.  Singleton is just a fancy
name for global variable.  We should call it the Global Variable
(anti)Pattern.

I can not agree more.

If the singleton is not going to be used before or after main(), then
the following is enough:

// header.h start
class X { /* ... */ };
extern X global_x;
// header.h end

If the singleton should be usable before and after main(), then there
is Schwarz Counter idiom to initialise your global variables, just the
same way as std::cout, std::cin, ... globals get initialised:

// header.h start
class X { /* ... */ };
extern X* global_x;

struct X_init
{
X_init();
~X_init();
}
// Put a copy of x_init (const === static)
// in every translation unit that includes header.h
const x_init;
// header.h end

// source.cc start
// These PODs are zero-initialised before
// any constructors are invoked.
static int counter;
X* global_x;

X_init::X_init()
{
if(!counter++)
global_x = new X;
}

X_init::~X_init()
{
if(!--counter)
delete global_x;
}
// source.cc end
 
M

Maxim Yegorushkin

And there are valid reasons for using it.

What are those reasons?
Some things must be inherently unique in a process.

This is what global variables are for. C++ has enough means to make
sure your globals are initialised before the first use.
Whatever::instance() is inconvenient and unnecessary.
 
S

Stefan Ram

Alan Johnson said:
1. Don't use singletons. Ever. Pretty much all of the value of the GoF
Design Patterns book is negated by the fact that they chose to
legitimize Singleton as a design pattern. Singleton is just a fancy
name for global variable. We should call it the Global Variable
(anti)Pattern.

In the C++ "Hello world!" program, usually »::std::cout« is used,
which seems to be a »global variable« to me.
(In Java, »java.lang.System.out« has the same rôle.)

Do you see a way to get rid of it (Possibly by a redesign of
the library)?

By a redesign of the language we could use:

int main( ::std::env & env )
{ env.out << "Hello World!\n"; }

This env would have to be passed to every function that wants
to print something to env.out. Assume that f and g do not need
to print something:

int main( ::std::env & env )
{ f(); }

void f(){ g(); }

void g(){ }

However, if one detects later that one wants to use »env.out«
in g, one not only needs to change g, but the whole »call chain«
(here: f), to allow this:

int main( ::std::env & env )
{ f( env.out ); }

void f( ::std::eek:stream & stdout ){ g( stdout ); }

void g( ::std::eek:stream & stdout ){ stdout << "alpha"; }

Possibly, one might say that »env« has low cohesion, because
it is a collection of different things, which only share that
they used to be global before.
 
N

Noah Roberts

Maxim said:
What are those reasons?


This is what global variables are for.

Globals do not offer any protection against further instantiation. If
something must be unique then it should only be possible to create a
single variable of that type. How are you going to offer that with an
unprotected variable, a comment saying, "Don't make any of these..."?

That method does not work. People who think they need to instantiate it
again may do so anyway. Singletons offer a design based solution that
is enforced in the code. Design is always best enforced by the code
where possible. It keeps the hacks to a minimum.
 
N

Noah Roberts

anon said:
(e-mail address removed) wrote:

Where do you unlock the mutex?

If the OP is following the same design as the future C++ api (seen in
boost::thread versions after 1.35 I believe) then it is unlocked when
scope is left.
You do not have a compilable code, but if instance is global variable,
then it is initialized before entering main(). Therefore, before any
thread is created

Not if you create a thread globally. The future C++ api for instance
will create threads on construction of a thread variable.
 
N

Noah Roberts

Alan said:
anon said:
On Dec 3, 1:47 am, Alan Johnson <[email protected]> wrote:

[lots of cutting]
1. Don't use singletons. Ever. Pretty much all of the value of the
GoF Design Patterns book is negated by the fact that they chose to
legitimize Singleton as a design pattern. Singleton is just a fancy
name for global variable. We should call it the Global Variable
(anti)Pattern.
greatly simplifies a lot of application code. And you are correct, in
this case it basically is, in fact, a global variable.

Both of you are not correct. The singleton is not a global variable.

TheClass* instance = NULL; TheClass * GetInstance() { if ( NULL ==
instance ) { instance = new TheClass; } return instance; }

In this case, the instance is created when the function GetInstance()
gets called the first time, while global variables gets created
before entering main()

That still has all the negative effects of a global variable. To name a
few
= hidden dependencies in functions that use it
= persistent global state
- tight coupling between anything that uses it

Explain why these are problems. Where are the *hidden* dependencies?
If persistent global state is needed then why enforce a policy that
dictates you don't have persistent global state? How are you to remove
coupling between objects that use each other? Is there some extra form
of coupling you think exists here?
It also brings its own set of new problems.
= multiple responsibilities in one class (responsible for limiting the
number of instances and for whatever the class is normally supposed to do

It's a minor point that can be fixed through the use of policies. See
Modern C++ Design by Alexandrescu.
= the whole cluster of problems around thread-safe lazy initialization

Yes, threads introduce problems.
- someday you will want more than one (you always do)

You don't use singleton because you think you'll only want one. You use
singleton because there can be only one.
- typically unable to derive from the singleton class (without heroic
effort)

Who says?
You would typically be better off just declaring a global
variable and using that instead.

Once again, this does not protect against the creation of another
variable of that type.
 
J

jason.cipriani

On Dec 3, 1:47 am, Alan Johnson <[email protected]> wrote:

[lots of cutting]
greatly simplifies a lot of application code. And you are correct, in
this case it basically is, in fact, a global variable.

Both of you are not correct. The singleton is not a global variable.

TheClass* instance = NULL;
TheClass * GetInstance()
{
   if ( NULL == instance )
   {
     instance = new TheClass;
   }
   return instance;

}

In this case, the instance is created when the function GetInstance()
gets called the first time, while global variables gets created before
entering main()

Juts to pick nits, in my particular situation I'm instantiating the
object at global scope, so it is initialized before main(), not on
first demand:

static TheClass *instance = new TheClass();

TheClass * GetInstance () {
return instance;
}


Jason
 
J

jason.cipriani

If you are going to declare it as a global variable, then why go through
the trouble of hiding it behind a function and calling it a singleton?

Mostly organization and personal preference. I'd rather keep the
"getInstance" as a static member of the relevant class, and I just
prefer to access it through a function rather than making it a public
static member of the class. There's no functional reason, though.

Jason
 
J

jason.cipriani

If the OP is following the same design as the future C++ api (seen in
boost::thread versions after 1.35 I believe) then it is unlocked when
scope is left.

Yes. Those Mutex/MutexLocker classes don't actually exist, they were
just for the example. I figured they'd be idiomatic but I guess I
should have been clearer.

Jason
 

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

No members online now.

Forum statistics

Threads
473,764
Messages
2,569,567
Members
45,041
Latest member
RomeoFarnh

Latest Threads

Top