Search code examples
c++multithreadinglockingstdatomicdouble-checked-locking

LazyArray template via std::unique_ptr, is this a proper implementation of the double-check idiom?


Does this code safely implement the double-check idiom using C++11 atomic? I saw in "The C++ Programing Language 4th ed." an example that uses atomic<bool> and I did my best to keep things equivalent but I'm not confident. Also, can this be improved?

I'd like to avoid call_once because of the storage overhead of once_flag.

This "LazyArray" was written as a memory-reduction effort to replace arrays with minimum changes to client code (only a small subset of the elements are expected to be accessed). Access to the array from multiple threads is a fact of life and locking broadly would be problematic due to performance.

/**
 * Lazy creation of array elements.
 * They will only be created when they are referenced.
 *
 * This "array" does not support iteration because it can have holes
 *
 * Array bounds isn't checked to keep it fast.
 * Destruction of the array destroys all the T objects (via the unique_ptr d'tor)
 */

template<class T, size_t size>
class LazyArray
{
    typedef LazyArray<T, size> mytype;
public:
    // copying is not allowed (unlike regular arrays)
    LazyArray(const LazyArray&) = delete;
    LazyArray& operator=(const LazyArray&) = delete;

    LazyArray(){}

    T& operator[](size_t i)
    {
        return at(i);
    }

    const T& operator[](size_t i) const
    {
        return const_cast<mytype *>(this)->at(i);
    }
private:
    using guard = std::lock_guard<std::mutex>;

    // get T object at index i by reference
    T& at(size_t i) // only non-const variant is implemented, const version will use const_cast
    {
        auto &p = m_array[i];
        std::atomic<T*> ap(p.get());

        if(!ap) // object not created yet
        {
            guard g(mtx);
            ap = p.get();
            if(!ap)
                p.reset(new T);
        }
        return *p;
    }

    std::unique_ptr<T> m_array[size];
    std::mutex mtx;
};



Solution

  • No, in theory.

    The check:

            std::atomic<T*> ap(p.get());
            if(!ap) // object not created yet
    

    Should be acquire corresponding to this release:

                    p.reset(new T);
    

    In fact it is not.

    How it may fail:

    1. Some parts of object construction new T may be visible to the other thread after it sees p assigned with new value
    2. The assignment p.reset may be torn since it is non-atomic

    Possibly, in some platforms.

    1. This happens either if stores can be reordered after other stores (does not happen on x86) or if compiler decides to swap stores these stores itself (not likely)
    2. Pointer-sized variable writes are not torn

    Use std::atomic to fix both issues:

       std::atomic<T*> m_array[size];
    

    Sure you have to delete T* manually. I'd suggest creating atomic_unique_ptr based on std::atomic<T*> and using it there