Simple Array add() crashs randomly


#1

I have some code that seems to crash from time to time. It adds 32 samples from the beginning of the array and adds them at the end:

void addOverlapSamples(Array *sampleData)
{
int count = 0;
while (count < 32 && sampleData->size() > count)
{
sampleData->add(sampleData->getReference(count));
count++;
}
}

The error stack:

Thread 29 Crashed:
0x0000000118541e86 juce::Array<float, juce::DummyCriticalSection, 0>::add(float const&) + 62

I’m almost sure that i haven’t any multi threading issues because i don’t see any other thread that does access the array or is near that code. And i also have a lock that avoids this.

My guess is that it has something to do with the memory allocation inside the array. Can this be and do i have to copy the values to a temporary array to avoid this?

Any help is welcome.


#2

Think about what you’re doing:

  1. You get a reference to an address in the array
  2. You ask the array to add another element
  3. The array has run out of space, so it re-allocates internally, meaning that the address you got earlier is now invalid
  4. You give it the invalid address to read its new item from!

#3

If you size the array correctly to start with I think you can be guaranteed not to have it move your shit underneath you. And it might go faster.


#4

Ok i see, then it has something to do with the internal resizing and memory allocation of the array. Thanks for the answers. Note to me: c++ is not the same as c# even with a great library like juce :slight_smile:

I will use resize before adding the items or maybe i can even set the right size when i create the array.

edit: ensureStorageAllocated seems to be the right function for this.


#5

Usually when I’m working with signals I preallocate a buffer using AudioSampleBuffer for whatever I need rather than using a JUCE Array, and then work with the float * from there.

(An array add operation has to havean additional comparison for each add you do to check the size limit hasn’t been exceeded. It won’t much difference, but it’s there)


#6

That’s kind of a solution, but it’s a really bad one!

The problem is that it’ll always be undefined behaviour to take a pointer to an array, then mutate the array, and then use that pointer afterwards.

Sure, you can bodge things by pre-allocating so that the array won’t need to re-allocate when you perform the add, but it’s a hack and is still technically undefined behaviour.

The correct thing to do is just to take a by-value copy of that array element, then add it.


#7

Thanks for pointing this out. I will do this.


#8

@Jules … I don’t think it’s necessarily undefined behaviour (unless you’ve formalised how juce::Array<> works) for std::vector<> at least:


#9

OK, that’s fair enough for std::vector. It’s obviously basically the same for Array, but we don’t have quite such a formal guarantee :wink:

However… Given a move operator, it’s actually not really any overhead to just do a value copy + add via move, which also makes your code simpler, as you don’t need to bother pre-allocating space.


#10

Ok - agreed - especially as in this instance the worst case is a doubling in the array size, and therefore presumably only a single additional allocation if Array<> does the doubling size expansion thing each time?

But if this is in an audio callback I’d have gone for the preallocation anyway to be on the safe side.

And I expect we are dealing with floats so it’s the same deal performance wise anyway with or without a move operator. Is that right?

void addOverlapSamples(Array *sampleData)
{
  int count = 0;
  while (count < 32 && sampleData->size() > count)
  {
    sampleData->add(sampleData[count]);
    count++;
  }
}

I’ve got another problem with it though - I think - for some reason the loop written like that is difficult for me to reason about.

But if i assume sampleData->size() != 0. In this case sampleData->size() increases by one for every iteration of the loop. As does count. So sampleData->size() is always greater than count.

So making some assumptions about what was intended, maybe it’d be easier to follow as:

void addOverlapSamples(Array *sampleData)
{
  int samplesToRepeat = jmin(sampleData->size(), 32); 

  for (int i = 0; i < samplesToRepeat; ++i)
    sampleData->add(sampleData[i]);
}