Re: Is this safe?
Joshua Maurice wrote:
On Jun 30, 11:46 am, mattb <matthew.b...@l-3com.com> wrote:
I have come across a singleton implementation of the following form in
some code I have to maintain...
[Out of order quote]
Is this safe?
Not in the slightest.
Are you really sure about slightest ? (see below)
file - Singleton.h
#ifndef SINGLETON_H
#define SINGLETON_H
You may want a more unique name for the macro, something involving
your company name for example. I'd imagine SINGLETON_H would be a
common name.
Also, "xxx.hpp" is the way of C++, not "xxx.h". Just a stylistic
nitpick.
It's perhaps often used for header files with only templates inside. But
is that an official recommendation of the C++ standard ? I don't think so.
See "C++ and the Perils of Double Checked Locking"
http://www.aristeia.com/Papers/DDJ_Jul_Aug_2004_revised.pdf
for a thorough description of why the above does not work and has
never worked.
It depends perhaps (?) on the CPU / OS, but I think the synchronization
primitives should work equally on each CPU, since safe multi threaded
code couldn't be written safely without memory fences.
But to play it safe [for me ;-)] I refer to Windows with a x86 / x64 CPU.
The relevant code, which IMHO makes the code safe (beside perhaps one
detail) is:
AccessLock<CriticalSection> access(key_);
if ( !instance_ )
{
static Singleton<T> theInstance;
instance_ = &theInstance;
}
Let me rewrite the code, how I interpret it to work (internally):
a) CriticalSection.EnterCriticalSection();
b) if (!instance) {...}
c) CriticalSection.LeaveCriticalSection();
At point a) c) memory barrier instructions are executed, which ensure
that no reordering occurs inside the boundaries and that all changes to
memory are visible to all cores.
What perhaps might be a (although I suppose it to be very unlikely)
problem is the second
Which could be an register operation only, not accessing the memory.
Either a volatile modifier should be added or under Windows an
InterlockedExchange instruction should be used to play it 100% safe.
[...]
Also, it gets even worse. You have static data members of template
classes. It doesn't work the way you want it to work. You have to
define static data members in a single translation unit (read: cpp
file). You cannot declare them in the header file, and you definitely
cannot have "static" in "template<typename T> static Singleton<T>*
Hm, since you can't export templates easily (without using export), how
should a static member template be initialized, if not in the header file ?
[...] whch contains a template instantiation for every argument to the
template used in the entire program.) Short version: avoid static data
For each instance separately ?
I have several compilers which compile such code (initialization of
static template members in the header file) and AFAIK/IIRC the linker is
required to remove all double template instances - I'm not 100% sure
about if the standard requires that too.
I remember having tracked down a compiler bug, which hasn't done this
properly [removing double instances]. But today the code should be safe
on recent compilers.
[...]
Here's an example of how to do a singleton correctly. It avoids
The code seems to be correct, with the restrictions you made. Besides
that my compiler would complain about static members in functions not to
be thread safe.
The difference to the OP's code is that the singleton is allocated
always in your examples and that your code is perhaps more portable if
no concurrent access during startup is required. The only question is
how can this be assured ?
[...]
Andre
--
[ See http://www.gotw.ca/resources/clcm.htm for info about ]
[ comp.lang.c++.moderated. First time posters: Do this! ]