Race condition in Dune::Singleton::instance()
In flyspray/FS#1718 (comment 20700) @carsten.graeser asks whether Dune::Singleton
is thread-safe. So I had a look, and it is not thread-safe.
From singleton.hh
DUNE_EXPORT static T& instance()
{
/* Smartpointer to the instance. */
static std::unique_ptr<T> instance_;
if(instance_.get() == 0)
instance_ = std::unique_ptr<T>(new T());
return *instance_;
}
There is a chance for two threads to enter instance()
concurrently. Then there is a reader-writer-race between one thread's instance_.get()
and the other thread's instance = ...
, and a writer-writer race between both threads' instance = ...
if this is the first time the instance method was called.
A better way to achieve the same thing is:
DUNE_EXPORT static T& instance()
{
static T instance_;
return instance_;
}
In this case the compiler ensures that the constructor for the instance_
variable is called only once. If multiple threads enter instance()
concurrently, one of them will invoke the constructor, while the others will wait. If the constructor throws, one of the waiting threads will retry it, until one of them succeeds. Once the constructor succeeds, the compiler will mark the variable as initialized and any threads still waiting (and any later invocation of instance()
will skip the constructor call. The effect is as if the constructor call is wrapped in a std::call_once()
, with the once_flag
argument automatically supplied by the compiler.
If the constructor of T
is simple enough, the compiler may even be able to do static initialization, and skip the synchronization needed for the implicit call_once altogether.