This pattern is most often used in the instance() method of a Singleton (nevermind that Singletons themselves tread mighty close to the dark side!) DCLP looks something like:
Filbert * Filbert::instance()
{
if (instance_ == 0)
{
MutexGuard guard(instanceMutex_);
if (instance == 0)
instance_ = new Filbert();
}
return instance_;
}
The theory being that once you have your instance you never again need to lock the mutex.
The article raised two issues:
1) In a dual processor system with caching, the processor that doesn't create the Filbert could see the value of instance_ but still have cached, stale, information about the memory containing the new Filbert.
2) Even in a single processor system, the compiler is allowed to reorder statements as long as the result appears to be the same as one would see in a single processor, single threaded environment. Strange as it may seem, the compiler is allowed to store the address of the new Filbert into instance_ before calling the constructor of the new object.
#2 is easily resolvable. The root of the problem is that instance_ is overloaded -- serving both as a pointer to Filbert and as a bool indicating that Filbert has been created. Separating these two responsibilities produces the "triple checked locking pattern" (TCLP)
Filbert * Filbert::instance()
{
if(instance_is_valid_)
{
MutexGuard guard(instanceMutex_);
if(instance == 0)
instance_ = new Filbert();
else
instance_is_valid_ = true;
}
return instance_;
}
Instance_is_valid_ is set only when thread one creates the Filbert and exits from the scope of the mutex, then thread 2 enters the scope of the mutex to find that the instance has already been created. I believe that this is safe on a single processor system for any legal reordering of the code. Alas, it still doesn't address the problem on a multi-processor system where each processor has its own cache.
It'll be interesting to see how cache is configured on the new multi-core processor chips that AMD & Intel are talking about.
The remaining problem with both DCLP and TCLP is that there is no way to delete the Filbert safely so resource leaks are gonna happen. Since the the need to delete usually comes at end-of-process time this is a non-issue on many (but not all) platforms (except for the purists and those running leak detection tools.) but still it would be nice to do it right.
2 comments:
It seems suspicious to me why first "if" checks instance_is_valid_. Maybie it should check
if (! istance_is_valid_) ?
Filbert * Filbert::instance()
{
if( instance_is_valid_)
{
MutexGuard guard(instanceMutex_);
if(instance == 0)
instance_ = new Filbert();
else
instance_is_valid_ = true;
}
return instance_;
}
You're right, Piotr, It should check NOT instance_is_valid.
Post a Comment