Is field AudioVisualiserComponent::ChannelInfo::levels thread safe in example AudioRecordingDemo?

1

Hi, I have been studying the code for AudioRecordingDemo and I am having trouble seeing how the field Array<Range<float>> levels; is thread safe. It is accessed in pushSample from the thread “JUCE ALSA”. It is also accessed by the thread “JUCE Message Thread” in function AudioVisualiserComponent::paint which calls paintChannel.

2

Array.getReference() holds a scoped lock, which means both threads wait for each other when needed and thus the race condition is avoided.

3

However, the default lock is a DummyCriticalSection, which doesn’t lock…

template<typename ElementType, typename TypeOfCriticalSectionToUse = DummyCriticalSection, int minimumAllocatedSize = 0>
class Array< ElementType, TypeOfCriticalSectionToUse, minimumAllocatedSize >

DummyCriticalSection:

A class that can be used in place of a real CriticalSection object, but which doesn’t perform any locking.

The idea in a FIFO is you push on one end and pop from the other end avoiding a race condition.
The shared memory is std::atomic<int> nextSample in ChannelData.

N.B. it is not a FIFO strictly speaking. It is just historical data, and the drawing reads from the write position backwards.

1 Like
4

True, in this particular case there is no need for the Array to have a “CriticalSection” type locking interface on the template - which you can add to the Template declaration to override DummyCriticalSection, if you need to be sure - because the AudioRecordingDemo is using its own ScopedLock (“writerLock”), so its not necessary to set it in the Array() declaration.

Its just key to point out that the Array class is set up to allow you to change the lock type, according to needs, and as well the AudioRecordingDemo uses writerLock separately anyway … so there are no thread-safety issues here, its just a matter of digging a little deeper in to the declarations.

5

Sorry to nit pick again:
The lock on the Array, if activated, would only guard for inserting or removing elements.
It doesn’t help when accessing elements inside the array. It is not safe to access elements from different threads.
But that’s not happening here anyway.

1 Like
6

Are you referring to the writerLock in AudioRecorder?

7

I’m glad you noticed the DummyCriticalSection, it’s key to understanding how this works. So to me the only part of the code that is thread safe is the std::atomic<int> nextSample { 0 }, subSample { 0 };.

Inside ChannelInfo::pushSample the only part that is thread safe is the ++nextSample. This function is accesed by the thread “JUCE ALSA”, which is where the sample comes from.

Inside the “JUCE Message Thread” eventually the paint method is called. Inside the paint method is

        paintChannel (g, r.removeFromTop (channelHeight),
                      c->levels.begin(), c->levels.size(), c->nextSample);

to me it looks like the accessing of c->nextSample is the only part thread safe because the nextSample is being passed in by value. If you follow that paintChannel call you will see:

auto level = -(levels[(nextSample + i) % numLevels].getEnd());
...
path.lineTo ((float) i, -(levels[(nextSample + i) % numLevels].getStart()));

These accesses are not thread safe when it comes to the data though and I feel like nextSample could change while they are being called.