[RESOLVED] Array move semantics and thread safety

Hi All,

I’m using a juce::Array with a juce::CriticalSection as its TypeOfCriticalSectionToUse template parameter, and I’d like to know whether the move semantics of Array are thread safe.

The Array (Array&& other) constructor doesn’t lock at all:

Array (Array&& other) noexcept
    : values (std::move (other.values))
{
}

and Array& operator= (Array&& other) locks the LHS array, but not the RHS

Array& operator= (Array&& other) noexcept
{
    const ScopedLockType lock (getLock());
    values = std::move (other.values);
    return *this;
}

I’m using std::move to quickly make a local copy of an Array on my main thread and leave the original Array empty.

Array<float, CriticalSection> progressUpdates = std::move(mySharedArray);

In my case Array::swapWith actually works nicely:

Array<float, CriticalSection> progressUpdates;
mySharedArray.swapWith (progressUpdates);

But I was confused about why std::move wouldn’t be thread safe. I had run into situations using the move semantics where the RHS / source array would be modified in the midst of the move, causing undefined behavior.

what is the underlying type of values? is it HeapBlock? perhaps look at underlying type’s move ctor/assign functions and see if they lock or are thread-safe?

Adding CriticalSection to Array was something I did very early in juce’s history, and have regretted since - I’d very much recommend not using it like that, and we should probably deprecate the feature at some point.

The std library people thought ahead properly when they designed their container classes, and sensibly had a policy of keeping locking completely separate from the containers themselves, thus avoiding the many edge-cases like the one you’re hitting.

How can an operation involving two objects, each with a lock, where one of them is being deleted (along with its lock) possibly be made thread-safe? Even if we could bodge that, it’s a horrible programming pattern!

One array with a lock seems kind of handy for simple add/remove operations, but as soon as it has to interact with anything else that’s also locked, then you need to stop using the array’s internal lock, and create your own one outside, where it’s used explicitly by all the code that interacts with it.

2 Likes

That makes sense, thanks for the response. I’ll avoid the templated CriticalSection and use one that I declare - that seems easier to control and avoids using more locks than I need.

Looking at my code now there’s no need for progressUpdates to have a CriticalSection at all… but that’s a different issue!

  • John