How to atomically copy types that are not-trivially copyable

Question

I'm writing an Atom class, for types T are not trivially-copyable. I was wondering if my below implementation of load() and store() could cause a race condition.

class Atom {
    // Assumptions:
    // 1. atomic<T> is not natively supported
    // 2. T does not contain any interior-pointers (pointers pointing to a field
    //    in the structure or the struct itself)
    // 3. T is neither trivially-copyable nor trivially-destructible
    // 4. T's copy constructor is thread-safe; i.e. The object being copied is not
    //    mutated without synchronization.

    using LT = typename std::aligned_storage<sizeof(T), alignof(T)>::type;

    spin::MRSWLock<T> locked_object;  // a multiple-reader-single-writer spinlock

    template<class... Args, class = std::enable_if_t<std::is_constructible_v<T, Args...>>>
    explicit Atom(Args&&... args): locked_object(std::forward<Args>(args)...) {}

    T load() const {
        LT l;
        {
            auto x = locked_object.read(); // get read lock

            // make a bitwise/shallow copy of object
            l = *reinterpret_cast<const LT*>(&*x);

        } // object unlocked here when x goes out of scope

        // make a deep copy of the object here by calling copy constructor.
        return T(*reinterpret_cast<const T*>(&l));
    }

    template<class... Args, class = std::enable_if_t<std::is_constructible_v<T, Args...>>>
    void store(Args&&... args) const {
        LT l, m;
        // construct the new object
        new (&l) T(std::forward<Args>(args)...);
        {
            auto x = locked_object.write(); // get write lock

            // make a bitwise-copy of the current object
            m = *reinterpret_cast<const LT*>(&*x);

            // make bitwise-assign of the new value of the object
            *reinterpret_cast<LT*>(&*x) = l;

        }// object unlocked here as x goes out of scope

        // destroy old object here.
        reinterpret_cast<T*>(&m)->~T();
    }
};

If a race condition is possible, is there a way to copy the object without calling its copy constructor while it's locked?

I would suggest using valgrind tool helgrind or the Intel Thread checker. — Damian Dixon, Mar 02 '21 at 12:54

score 0 · Answer 1 · answered Mar 02 '21 at 04:06

Your code is full of errors.

m = reinterpret_cast<const LT*>(&*x);

here you assign a pointer to a non-pointer.

l = reinterpret_cast<const LT*>(&*x);

and again.

If I attempt to read your mind and ignore your code and instead follow the comments, no, using a bytewise copy of an object as an object that is not trivially copyable is UB. Period.

Copying an object that is not trivially copyable requires running its copy constructor.

Destroying an object that isn't there

reinterpret_cast<T*>(&m)->~T();

is also UB.

You appear to be falling into the trap that only naive mapping to assembly instructiins determines the meaning and correctness of C++ code.

C++ is defined in terms of an abstract machine, and that machine knows where objects are and what their types are. This information may not be fully known to the compiler, it may not directly show up in assembly produced, but violating its rules is UB, and compilers can and do use that information to change what assembly is produced in ways that break your programs if you break the rules.

The worst part is that you don't know which compiler update or flag will break your code that passed your tests, possibly in insane ways like time travel. (Yes, in C++, UB is permitted to time travel and break code before it runs).

Don't use UB. It costs too much to maintain.

Thanks for letting me know, I forgot to dereference after casting in the assignment. ``` l = *reinterpret_cast(&*x); ``` ``` m = *reinterpret_cast(&*x); ``` I'm aware that the copy constructor has to be called to copy objects that are not trivially copyable. In my code, I was trying to only perform a bitwise-copy of the object (since we've assumed that the object contains no interior pointers) while the object is locked, and then perform a deep copy by calling the copy constructor after unlocking it. — Samuel Okechukwu, Mar 02 '21 at 04:33
@samu Yes, you are trying to perform UB. The standard places no requirements on the behaviour of code that runs UB. — Yakk - Adam Nevraumont, Mar 02 '21 at 04:49

How to atomically copy types that are not-trivially copyable

1 Answers1