Thread Sanitizer detects a data race in AudioThumbnail

#1

In latest develop (c96bf71488a), running the JUCE demo’s audio player demo with Thread Sanitizer (“Xcode/Edit Scheme/Run/Diagnostics/Thread Sanitizer”), when opening an audio file -

A data race is detected at AudioThumbnail::setLevels where it updates totalSamples.

(my env: Xcode 10.1, macOS 10.14.3)

This may be benign (am not familiar with the code) but I think that it’s good practice to either solve the race or mark it as benign, so people could use the invaluable Thread Sanitizer without time consuming false positives.

Cheers, Yair

6 Likes

#2

Thanks @ed95 for fixing this, that was quick! (Btw could had been nice to be updated about it in this thread)

When verifying that this was fixed I noticed that the AudioPlaybackDemo has another data race (tested on latest commit ff613d900a37), to get to this race you also need to play an audio file. It doesn’t necessarily happen immediately but with a few min long mp3 it happens to me after a few seconds.

Cheers, Yair

0 Likes

#3

One extra (possibly benign) issue which still exists after the fix - setDataSource() reads totalSamples in its return statement without any lock acquired. This means that another thread might perform setLevels() (since initialise() was already called) which could write into totalSamples before setDataSource()'s return statement gets to read it - thus affecting the return value.

This might be theoretical, but it would probably be more robust to move setDataSource()'s return statement into the scope of the lock acquired in the else clause (and add the same return statement to the end of the if clause as well). Or some similar fix.

Thanks,
Dan

3 Likes

#4

OK, I’ve pushed a few more fixes for things that tsan flagged up here + your suggested fix @danradix. The data race flagged up just after the file starts playing should be fixed here, it’s making it’s way through our CI at the moment but should be on develop in a few minutes.

1 Like