Re: Singleton_pattern and Thread Safety

From:

Joshua Maurice <joshuamaurice@gmail.com>

Newsgroups:

comp.lang.c++

Date:

Fri, 10 Dec 2010 19:42:13 -0800 (PST)

Message-ID:

<be03438d-f052-4ab9-a715-f454eb65cc65@i25g2000prd.googlegroups.com>

On Dec 10, 7:40 pm, Joshua Maurice <joshuamaur...@gmail.com> wrote:

On Dec 10, 7:17 pm, Leigh Johnston <le...@i42.co.uk> wrote:

On 11/12/2010 03:12, Joshua Maurice wrote:

On Dec 10, 6:38 pm, Leigh Johnston<le...@i42.co.uk> wrote:

On 11/12/2010 02:23, Leigh Johnston wrote:

On 10/12/2010 23:31, Ian Collins wrote:

On 12/11/10 10:08 AM, Leigh Johnston wrote:

On 10/12/2010 20:39, Ian Collins wrote:

On 12/11/10 09:21 AM, Leigh Johnston wrote:

Not considering object destruction when designing *new* classes=

is bad

practice IMO. Obviously there may be problems when working with
pre-existing designs which were created with a lack of such
consideration.

A programmer seldom has the benefit of a green field design. Eve=

n when

he or she does, there are still the dark and scary corners of th=

language where undefined behaviour lurks. Order of destruction
issues is
one such corner, especially when static objects exist in multipl=

compilation units.

I am well aware of the unspecified construction/destruction order
associated with globals in multiple TUs and that is primary reaso=

n why

this method of James's should be avoided. The order of destructio=

n of

"Meyers Singleton" objects *is* well defined for example although=

making

the "Meyers Singleton" method thread safe is not completely trivi=

al.

That is another pattern I use, but as you say, it has issues of it=

s own.

Normally I instantiate all my singletons up front (before threading=

) but

I decided to quickly roll a new singleton template class just for t=

fun of it (thread-safe Meyers Singleton):

namespace lib
{
template<typename T>
class singleton
{
public:
static T& instance()
{
if (sInstancePtr != 0)
return static_cast<T&>(*sInstancePtr);
{ // locked scope
lib::lock lock1(sLock);
static T sInstance;
{ // locked scope
lib::lock lock2(sLock); // second lock should emit memory barrier h=

ere

sInstancePtr =&sInstance;
}
}
return static_cast<T&>(*sInstancePtr);
}
private:
static lib::lockable sLock;
static singleton* sInstancePtr;
};

template<typename T>
lib::lockable singleton<T>::sLock;
template<typename T>
singleton<T>* singleton<T>::sInstancePtr;
}

Even though a memory barrier is emitted for a specific implementatio=

n of

my lockable class it obviously still relies on the C++ compiler not
re-ordering stores across a library I/O call (acquiring the lock) bu=

t it

works fine for me at least (VC++). I could mention volatile but I
better not as that would start a long argument. Roll on C++0x.

If I'm reading your code right, on the fast path, you don't have a
barrier, a lock, or any other kind of synchronization, right? If yes,
you realize you've coded the naive implementation of double checked?
You realize that it's broken, right? Have you even read
http://www.aristeia.com/Papers/DDJ_Jul_Aug_2004_revised.pdf
?
To be clear, this has undefined behavior according to the C++0x
standard as well.

I am aware of double checked locking pattern yes and this is not the
double checked locking pattern (there is only one check of the pointer
if you look). If a pointer read/write is atomic is should be fine (o=

the implementation I use it is at least).

You've hidden the second check with the static keyword.

Example: Consider:

  SomeType& foo()
  {
    static SomeType foo;
    return foo;
  }

For a C++03 implementation, it's likely implemented with something
like:

  SomeType& foo()
  {
    static bool b = false; /*done before any runtime execution, sto=

red

in the executable image */
    static char alignedStorage[sizeof(SomeType)]; /*with some magic
for alignment */
    if ( ! b)
      new (alignedStorage) SomeType();
    return * reinterpret_cast<SomeType*>(alignedStorage);
  }

That's your double check.

For C++0x, it will not be implemented like that. Instead, it will be
implemented in a thread-safe way that makes your example entirely
redundant.

Err, that should be:

  SomeType& foo()
  {
    static bool b = false; /*done before any runtime execution, stored
in the executable image */
    static char alignedStorage[sizeof(SomeType)]; /*with some magic
for alignment */
    if ( ! b)
    {
      new (alignedStorage) SomeType();
      b = true;
    }
    return * reinterpret_cast<SomeType*>(alignedStorage);
  }