Array = operator with move semantics causes leaks


#1

The Array = operator with move semantics sets the numUsed member of the other Array it swaps with to 0 which results in deleteAllElements() not calling the destructor on the elements when it gets deleted.

Array& operator= (Array&& other) noexcept
{
    const ScopedLockType lock (getLock());
    data = static_cast<ArrayAllocationBase<ElementType, TypeOfCriticalSectionToUse>&&> (other.data);
    numUsed = other.numUsed;
    other.numUsed = 0;
    return *this;
}

A similar thing happens in ArrayAllocationBase for the numAllocated member.

Causes leaks when doing stuff like this:

juce::Array<juce::String> createAnArray()
{
    juce::Array<juce::String> anotherArray;
    anotherArray.add (juce::String ("anotherArray"));
    return anotherArray;
}

juce::Array<juce::String> testArray;
testArray.add (juce::String ("testArray"));
testArray = createAnArray();

#2

Damn.. Thanks very much for spotting that. I think you're misunderstanding the problem, but there was a mistake in there which I've fixed now. (ArrayAllocationBase was correct though).


#3

I checked out the fix you did.

Just curious, howcome you call deleteAllElements() in the function instead of swapping the numUsed variables? The destructor gets called on the rvalue that was passed into the function which calls deleteAllElements() anyways. That's where I got the idea that ArrayAllocationBase was wrong as well since I figured it should swap its numAllocated with the rvalue passed into it as well.

Maybe there's something I'm missing?


#4

Well, it could be done with a swap, but since the ArrayAllocationBase has a move operator, it felt more appropriate to use that. It also feels better to me to delete the array's own elements before the function returns, rather than letting that happen inside a different Array object at some indeterminate time later, since in edge-case situations I guess it's not impossible that an item in the array may indirectly have a reference back to the array that contains it, and could use that in its destructor.