Reputation: 7963
I want to lazily instantiate a ViewportFactory
as follows:
ViewportFactory* RenderObjectFactory::GetViewportFactory() {
if (viewportFactory == nullptr) {
std::lock_guard<std::mutex> instantiationLockGuard { factoryInstantiationMutex };
if (viewportFactory == nullptr) {
switch (GetAndCommitRenderingAPISelection()) {
case 0:
ViewportFactory* v = new DirectXViewportFactory { };
viewportFactory = v;
break;
}
}
}
return viewportFactory;
}
Is that sufficiently threadsafe? My thinking is that by only assigning viewportFactory
once the new DirectXViewportFactory
is properly instantiated, it is...
Upvotes: 1
Views: 98
Reputation: 2825
The code as it stands is not threadsafe. The compiler is free to optimize the successive null checks since the pointer is never changed on the same thread.
I think the suggested way to do this with c++11 is with std::call_once or with a static variable. See here for an example. Also: reference
Upvotes: 1
Reputation: 15869
I don't believe this is safe. With optimization viewportFactory
could still be assigned a pointer to allocated but uninitialized memory before the constructor is called. See Example 6 in C++ and The Perils of Double-Checked Locking.
Even if it were safe, this code would not be maintenance-friendly. When someone comes along later, perhaps you or perhaps someone completely unfamiliar with it, it is very easy to add code in the wrong place that would then introduce a race condition. These bugs are difficult to reproduce and isolate - is the performance gain really worth that risk?
As @DieterLücking hints, lazy instantiation can be implemented with a local static variable, which was made thread-safe in the C++11 standard. You do have to be careful that your compiler actually implements this part of the standard - for Visual Studio (since you appear to be on Windows) this did not happen until Visual Studio 2013.
Upvotes: 1