MPESynthesiser::renderNextSubBlock() currently has a data race against all the voice-modifying functions like MPESynthesiser::clearVoices() etc. because it also uses the voices. I actually had crashes because of this. Adding
const ScopedLock sl (voicesLock);
to both renderNextSubBlock overloads as well as turnOffAllVoices (which also has this race) fixes the crashes for me.
In addition, there are other dodgy-looking locks like MPESynthesiserBase::noteStateLock, whose only purpose seems to be locking setCurrentPlaybackSampleRate against renderNextBlock but afaics they can’t be called concurrently anyway? It feels this could be simplified and the number of locks reduced, but at the moment this doesn’t cause me any observable problems (unlike the voicesLock issue described above).
Why not? You could quite easily be calling setCurrentPlaybackSampleRate() from the message thread whilst renderNextBlock() is being called on the audio thread, in which case I think the lock makes sense as it’s guarding against the MPEInstrument object being modified concurrently. Unless I’m missing something?
Hm, yes, I think technically you are correct. But that sounds like bad practice to me. The proper place to call setCurrentPlaybackSampleRate is prepareToPlay(), and afaik you’re guaranteed that this can’t be concurrent to a processing callback? (I might be wrong though)
And regardless of any of that: if that thing guards resetting the sample rate against the processing callback, why is it called noteStateLock?? What does it have to do with note states? And aren’t note states handled by another class? (MPEInstrument) I was staring at this for a while but couldn’t figure it out.
I vaguely remember that I actually have written large parts of this stuff myself, but it seems some things in there have changed since then because I don’t remember this lock being used in this way.
Haha, yes, see my post above. Maybe I’ll have some time later trying to remember it all and figure out what has changed since I introduced that lock (I would not have named it noteStateLock if it were not originally meant to guard the note state!).
Anyway, the other issue (voicesLock) is a genuine race condition causing crashes, so please fix
