fix crash enabling PFL after deleting EffectState while PFL is disabled

src/effects/defs.h

@@ -26,7 +26,12 @@ enum class EffectChainInsertionType {
     Num_Insertion_Types
 };
 
-const int kNumEffectsPerUnit = 4;
+constexpr int kNumEffectsPerUnit = 4;
+
+// NOTE: Setting this to true will enable string manipulation and calls to
+// qDebug() in the audio engine thread. That may cause audio dropouts, so only
+// enable this when debugging the effects system.
+constexpr bool kEffectDebugOutput = false;
 
 class EffectState;
 // For sending EffectStates along the MessagePipe

src/effects/effectsmanager.cpp

@@ -441,16 +441,24 @@ void EffectsManager::processEffectsResponses() {
 
 void EffectsManager::collectGarbage(const EffectsRequest* pRequest) {
     if (pRequest->type == EffectsRequest::REMOVE_EFFECT_FROM_CHAIN) {
-        //qDebug() << debugString() << "delete" << pRequest->RemoveEffectFromChain.pEffect;
+        if (kEffectDebugOutput) {
+            qDebug() << debugString() << "delete" << pRequest->RemoveEffectFromChain.pEffect;
+        }
         delete pRequest->RemoveEffectFromChain.pEffect;
     } else if (pRequest->type == EffectsRequest::REMOVE_CHAIN_FROM_RACK) {
-        //qDebug() << debugString() << "delete" << request->RemoveEffectFromChain.pEffect;
+        if (kEffectDebugOutput) {
+            qDebug() << debugString() << "delete" << pRequest->RemoveEffectFromChain.pEffect;
+        }
         delete pRequest->RemoveChainFromRack.pChain;
     } else if (pRequest->type == EffectsRequest::REMOVE_EFFECT_RACK) {
-        //qDebug() << debugString() << "delete" << pRequest->RemoveEffectRack.pRack;
+        if (kEffectDebugOutput) {
+            qDebug() << debugString() << "delete" << pRequest->RemoveEffectRack.pRack;
+        }
         delete pRequest->RemoveEffectRack.pRack;
     } else if (pRequest->type == EffectsRequest::DISABLE_EFFECT_CHAIN_FOR_INPUT_CHANNEL) {
-        //qDebug() << debugString() << "deleting states for input channel" << pRequest->channel << "for EngineEffectChain" << pRequest->pTargetChain;
+        if (kEffectDebugOutput) {
+            qDebug() << debugString() << "deleting states for input channel" << pRequest->DisableInputChannelForChain.pChannelHandle << "for EngineEffectChain" << pRequest->pTargetChain;
+        }
         pRequest->pTargetChain->deleteStatesForInputChannel(
                 pRequest->DisableInputChannelForChain.pChannelHandle);
     }

src/engine/effects/engineeffectchain.cpp

@@ -195,6 +195,23 @@ bool EngineEffectChain::disableForInputChannel(const ChannelHandle* inputHandle)
 
 // Called from the main thread for garbage collection after an input channel is disabled
 void EngineEffectChain::deleteStatesForInputChannel(const ChannelHandle* inputChannel) {
+    // If an output channel is not presently being processed, for example when
+    // PFL is not active, then process() cannot be relied upon to set this
+    // chain's EffectEnableState from Disabling to Disabled. This must be done
+    // before the next time process() is called for that output channel,
+    // otherwise, if any EngineEffects are Enabled,
+    // EffectProcessorImpl::processChannel will try to run
+    // with an EffectState that has already been deleted and cause a crash.
+    // Refer to https://bugs.launchpad.net/mixxx/+bug/1741213
+    // NOTE: ChannelHandleMap is like a map in that it associates an object with
+    // a ChannelHandle key, but it actually backed by a QVarLengthArray, not a
+    // QMap. So it is okay that m_chainStatusForChannelMatrix may be
+    // accessed concurrently in the audio engine thread in process(),
+    // enableForInputChannel(), or disableForInputChannel().
+    auto& outputMap = m_chainStatusForChannelMatrix[*inputChannel];
+    for (auto&& outputChannelStatus : outputMap) {
+        outputChannelStatus.enable_state = EffectEnableState::Disabled;
+    }
     for (EngineEffect* pEffect : m_effects) {
         if (pEffect != nullptr) {
             pEffect->deleteStatesForInputChannel(inputChannel);

src/engine/effects/message.h

@@ -10,11 +10,6 @@
 #include "effects/defs.h"
 #include "engine/channelhandle.h"
 
-// NOTE: Setting this to true will enable string manipulation and calls to
-// qDebug() in the audio engine thread. That may cause audio dropouts, so only
-// enable this when debugging the effects system.
-const bool kEffectDebugOutput = false;
-
 class EngineEffectRack;
 class EngineEffectChain;
 class EngineEffect;