Xenoprimate
Xenoprimate

Reputation: 7963

Double Locking Workaround

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

Answers (2)

sshannin
sshannin

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

rhashimoto
rhashimoto

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

Related Questions