guard against audio injector and local buffer datarace

libraries/audio-client/src/AudioClient.cpp

@@ -1168,16 +1168,18 @@ bool AudioClient::mixLocalAudioInjectors(float* mixBuffer) {
     memset(mixBuffer, 0, AudioConstants::NETWORK_FRAME_SAMPLES_STEREO * sizeof(float));
 
     for (AudioInjector* injector : _activeLocalAudioInjectors) {
-        if (injector->getLocalBuffer()) {
+        // the lock guarantees that injectorBuffer, if found, is invariant
+        AudioInjectorLocalBuffer* injectorBuffer;
+        if ((injectorBuffer = injector->getLocalBuffer())) {
 
             static const int HRTF_DATASET_INDEX = 1;
 
             int numChannels = injector->isAmbisonic() ? AudioConstants::AMBISONIC : (injector->isStereo() ? AudioConstants::STEREO : AudioConstants::MONO);
-            qint64 bytesToRead = numChannels * AudioConstants::NETWORK_FRAME_BYTES_PER_CHANNEL;
+            size_t bytesToRead = numChannels * AudioConstants::NETWORK_FRAME_BYTES_PER_CHANNEL;
 
             // get one frame from the injector
             memset(_localScratchBuffer, 0, bytesToRead);
-            if (0 < injector->getLocalBuffer()->readData((char*)_localScratchBuffer, bytesToRead)) {
+            if (0 < injectorBuffer->readData((char*)_localScratchBuffer, bytesToRead)) {
                 
                 if (injector->isAmbisonic()) {
 
@@ -1317,15 +1319,17 @@ void AudioClient::setIsStereoInput(bool isStereoInput) {
 }
 
 bool AudioClient::outputLocalInjector(AudioInjector* injector) {
-    Lock lock(_injectorsMutex);
-    if (injector->getLocalBuffer() && _audioInput ) {
-        // just add it to the vector of active local injectors, if 
-        // not already there.
-        // Since this is invoked with invokeMethod, there _should_ be
-        // no reason to lock access to the vector of injectors.
+    AudioInjectorLocalBuffer* injectorBuffer;
+    if ((injectorBuffer = injector->getLocalBuffer())) {
+        // local injectors are on the AudioInjectorsThread, so we must guard access
+        Lock lock(_injectorsMutex);
         if (!_activeLocalAudioInjectors.contains(injector)) {
             qCDebug(audioclient) << "adding new injector";
             _activeLocalAudioInjectors.append(injector);
+
+            // move local buffer to the LocalAudioThread to avoid dataraces with AudioInjector (like stop())
+            injectorBuffer->setParent(nullptr);
+            injectorBuffer->moveToThread(&_localAudioThread);
         } else {
             qCDebug(audioclient) << "injector exists in active list already";
         }
@@ -1333,7 +1337,7 @@ bool AudioClient::outputLocalInjector(AudioInjector* injector) {
         return true;
 
     } else {
-        // no local buffer or audio
+        // no local buffer
         return false;
     }
 }

libraries/audio/src/AbstractAudioInterface.h

@@ -33,7 +33,11 @@ public:
                                 PacketType packetType, QString codecName = QString(""));
 
 public slots:
+    // threadsafe
+    // moves injector->getLocalBuffer() to another thread (so removes its parent)
+    // take care to delete it when ~AudioInjector, as parenting Qt semantics will not work
     virtual bool outputLocalInjector(AudioInjector* injector) = 0;
+
     virtual bool shouldLoopbackInjectors() { return false; }
     
     virtual void setIsStereoInput(bool stereo) = 0;

libraries/audio/src/AudioInjector.cpp

@@ -51,6 +51,10 @@ AudioInjector::AudioInjector(const QByteArray& audioData, const AudioInjectorOpt
 {
 }
 
+AudioInjector::~AudioInjector() {
+    deleteLocalBuffer();
+}
+
 bool AudioInjector::stateHas(AudioInjectorState state) const {
     return (_state & state) == state;
 }
@@ -87,11 +91,7 @@ void AudioInjector::finish() {
 
     emit finished();
 
-    if (_localBuffer) {
-        _localBuffer->stop();
-        _localBuffer->deleteLater();
-        _localBuffer = NULL;
-    }
+    deleteLocalBuffer();
 
     if (stateHas(AudioInjectorState::PendingDelete)) {
         // we've been asked to delete after finishing, trigger a deleteLater here
@@ -163,7 +163,7 @@ bool AudioInjector::injectLocally() {
     if (_localAudioInterface) {
         if (_audioData.size() > 0) {
 
-            _localBuffer = new AudioInjectorLocalBuffer(_audioData, this);
+            _localBuffer = new AudioInjectorLocalBuffer(_audioData);
 
             _localBuffer->open(QIODevice::ReadOnly);
             _localBuffer->setShouldLoop(_options.loop);
@@ -172,7 +172,8 @@ bool AudioInjector::injectLocally() {
             _localBuffer->setCurrentOffset(_currentSendOffset);
 
             // call this function on the AudioClient's thread
-            success = QMetaObject::invokeMethod(_localAudioInterface, "outputLocalInjector", Q_ARG(AudioInjector*, this));
+            // this will move the local buffer's thread to the LocalInjectorThread
+            success = _localAudioInterface->outputLocalInjector(this);
 
             if (!success) {
                 qCDebug(audio) << "AudioInjector::injectLocally could not output locally via _localAudioInterface";
@@ -185,6 +186,14 @@ bool AudioInjector::injectLocally() {
     return success;
 }
 
+void AudioInjector::deleteLocalBuffer() {
+    if (_localBuffer) {
+        _localBuffer->stop();
+        _localBuffer->deleteLater();
+        _localBuffer = nullptr;
+    }
+}
+
 const uchar MAX_INJECTOR_VOLUME = packFloatGainToByte(1.0f);
 static const int64_t NEXT_FRAME_DELTA_ERROR_OR_FINISHED = -1;
 static const int64_t NEXT_FRAME_DELTA_IMMEDIATELY = 0;

libraries/audio/src/AudioInjector.h

@@ -52,6 +52,7 @@ class AudioInjector : public QObject {
 public:
     AudioInjector(const Sound& sound, const AudioInjectorOptions& injectorOptions);
     AudioInjector(const QByteArray& audioData, const AudioInjectorOptions& injectorOptions);
+    ~AudioInjector();
     
     bool isFinished() const { return (stateHas(AudioInjectorState::Finished)); }
     
@@ -99,6 +100,7 @@ private:
     int64_t injectNextFrame();
     bool inject(bool(AudioInjectorManager::*injection)(AudioInjector*));
     bool injectLocally();
+    void deleteLocalBuffer();
     
     static AbstractAudioInterface* _localAudioInterface;
 

libraries/audio/src/AudioInjectorLocalBuffer.cpp

@@ -11,8 +11,7 @@
 
 #include "AudioInjectorLocalBuffer.h"
 
-AudioInjectorLocalBuffer::AudioInjectorLocalBuffer(const QByteArray& rawAudioArray, QObject* parent) :
-    QIODevice(parent),
+AudioInjectorLocalBuffer::AudioInjectorLocalBuffer(const QByteArray& rawAudioArray) :
     _rawAudioArray(rawAudioArray),
     _shouldLoop(false),
     _isStopped(false),

libraries/audio/src/AudioInjectorLocalBuffer.h

@@ -19,7 +19,7 @@
 class AudioInjectorLocalBuffer : public QIODevice {
     Q_OBJECT
 public:
-    AudioInjectorLocalBuffer(const QByteArray& rawAudioArray, QObject* parent);
+    AudioInjectorLocalBuffer(const QByteArray& rawAudioArray);
 
     void stop();