I've just read the paper, and I believe that the following variation on
the code would work and would avoid the MP unsafe issues raised because
bool is defined to be a single byte.

Further-more, I'm pretty certain that it also resolves the issues with the
order of construction
and setting of the pointer in the singleton case, and probably resolves all
the other "over smart optimisation" issues as well

static volatile bool initialised=false;

if (!initialised)
{
CRYPTO_w_lock(CRYPTO_LOCK_XXX);
/* Avoid a race condition by checking again inside this lock */
if (!initialised)
{
x = ...;
initialised=true; // Atomic operation
}
CRYPTO_w_unlock(CRYPTO_LOCK_XXX);
}
/* Now, make use of x */

Or expressed in terms of the Singleton pattern:

in the header for the Singleton class file:

static volatile bool initialised;

in the Source file:

static volatile bool Singleton::initialised=false;

Singleton* Singleton::instance()
{
if (!initialised == 0) // 1st test
{
Lock lock;
if (!initialised) // 2nd test
{
pInstance = new Singleton;
initialised=true; // Atomic
}
}
return pInstance;
}

I've been using this approach for absolutely YEARS, and didn't realise
someone had honoured
it with a design pattern name!!!

I've copied this to Scott Meyers for him to comment on whether I've got this
right ...

Dave Partridge
-----Original Message-----
From: owner-openssl-dev@openssl.org
[mailtowner-openssl-dev@openssl.org]On Behalf Of Steven Reddie
Sent: 06 April 2005 10:02
To: openssl-dev@openssl.org
Subject: OpenSSL use of DCLP may not be thread-safe on multiple
processors


Hi All,

OpenSSL makes use of the DCLP (double-checked locking pattern) in a number
of places (rsa_eay.c and at least one engine; I haven't done an exhaustive
search), with code that usually looks like this:

if (x == NULL)
{
CRYPTO_w_lock(CRYPTO_LOCK_XXX);
/* Avoid a race condition by checking again inside this lock */
if (x == NULL)
{
x = ...;
}
CRYPTO_w_unlock(CRYPTO_LOCK_XXX);
}
/* Now, make use of x */

Some recent research I've done in this area, prompted by Scott Meyers' and
Andrei Alexandrescu's article "C++ and the Perils of Double-Checked Locking"
at http://www.aristeia.com/Papers/DDJ_J...04_revised.pdf, makes me
wonder whether this code is thread-safe on multi-processor machines. As the
article points out, DCLP is dangerous in general, however it is most likely
safe if the thing being tested and set is accessed atomically. On most
32-bit machines a 32-bit quantity will generally be accessed in a single bus
transaction, making it inherently atomic. However, there may be cases where
it is not atomic. An example could be on a machine that allows unaligned
accesses, such as the x86. It may be possible for half of the value to be
updated in another processors cache, and used (since the value is therefore
not NULL), before the other half is updated. It seems that in fact the race
condition that is trying to be avoided may have been reduced rather than
eliminated. While it may be true that the code generated by the compiler
doesn't typically result in unaligned accesses it is still a possibility
that exists, and there may be other ways for non-atomic access to occur
without unalignment being the cause.

I've tried some elaborate workarounds to maintain the optimisation that DCLP
provides, but they turn out to be not entirely safe on other processors such
as the Itanium. The easiest way to fix this would seem to be always
obtaining the lock before using the variables in question, but this could
have an impact on performance. A more involved alternative is to use locked
instructions, such as the Interlocked... Functions on Windows, and some
hand-rolled assembler on other platforms, to ensure that the values are
updated atomically. I'm not offering patches at this point in case there is
too much resistance to a performance hit, so I'm interested to know thoughts
either way. I agree that the margin for error is very, very small, and I
don't know how much of an impact on performance the necessary changes would
have, so I'm partly sending this so that if nothing is done and a future
race-condition is reported it may assist with locating the problem.

Regards,

Steven

__________________________________________________ ____________________
OpenSSL Project http://www.openssl.org
Development Mailing List openssl-dev@openssl.org
Automated List Manager majordomo@openssl.org



__________________________________________________ ____________________
OpenSSL Project http://www.openssl.org
Development Mailing List openssl-dev@openssl.org
Automated List Manager majordomo@openssl.org