decouple EffectStatesMapArray from EffectChain

This fixes a crash deleting EffectStates after starting up with an
effect unit assigned to multiple input channels.
EffectChain::enableForInputChannel kept reusing the same array for
EffectStates and putting a pointer to this on the effects MessagePipe.
That assumed that the engine had a chance to process the message to load
the states by calling EffectProcessorImpl::loadStatesForInputChannel
before the next call to EffectChain::enableForInputChannel. On startup,
before the engine starts, this assumption is not true. The result was
that EffectProcessorImpl ended up storing pointers to the same
EffectStates for multiple input channels. This caused a crash when
~EffectProcessorImpl tried to delete the EffectStates for the second
input channel because the EffectState had already been deleted when
deleting the EffectStates for the first input channel.

Fixes https://bugs.launchpad.net/mixxx/+bug/1740531

3 files changed, 17 insertions, 6 deletions

diff --git a/src/effects/effectchain.cpp b/src/effects/effectchain.cpp
index 594e225aa7..fe22be0be7 100644
--- a/src/effects/effectchain.cpp
+++ b/src/effects/effectchain.cpp
@@ -177,9 +177,9 @@ void EffectChain::enableForInputChannel(const ChannelHandleAndGroup& handle_grou
     // Allocate EffectStates here in the main thread to avoid allocating
     // memory in the realtime audio callback thread. Pointers to the
     // EffectStates are passed to the EffectRequest and the EffectProcessorImpls
-    // store the pointers. The EffectStatesMapArray and EffectStatesMap containers
-    // are owned by this EffectChain so they do not need be reallocated for each
-    // EffectRequest.
+    // store the pointers. The containers of EffectState* pointers get deleted
+    // by ~EffectsRequest, but the EffectStates are managed by EffectProcessorImpl.
+    auto pEffectStatesMapArray = new EffectStatesMapArray;
 
     //TODO: get actual configuration of engine
     const mixxx::EngineParameters bufferParameters(
@@ -187,7 +187,7 @@ void EffectChain::enableForInputChannel(const ChannelHandleAndGroup& handle_grou
           MAX_BUFFER_LEN / mixxx::kEngineChannelCount);
 
     for (int i = 0; i < m_effects.size(); ++i) {
-        auto& statesMap = m_effectStatesMapArray[i];
+        auto& statesMap = (*pEffectStatesMapArray)[i];
         if (m_effects[i] != nullptr) {
             for (const auto& outputChannel : m_pEffectsManager->registeredOutputChannels()) {
                 if (kEffectDebugOutput) {
@@ -205,7 +205,7 @@ void EffectChain::enableForInputChannel(const ChannelHandleAndGroup& handle_grou
             statesMap.clear();
         }
     }
-    request->EnableInputChannelForChain.pEffectStatesMapArray = &m_effectStatesMapArray;
+    request->EnableInputChannelForChain.pEffectStatesMapArray = pEffectStatesMapArray;
 
     m_pEffectsManager->writeRequest(request);
     emit(channelStatusChanged(handle_group.name(), true));
diff --git a/src/effects/effectchain.h b/src/effects/effectchain.h
index 160ce8b113..cedecdf80e 100644
--- a/src/effects/effectchain.h
+++ b/src/effects/effectchain.h
@@ -123,7 +123,6 @@ class EffectChain : public QObject {
     QList<EffectPointer> m_effects;
     EngineEffectChain* m_pEngineEffectChain;
     bool m_bAddedToEngine;
-    EffectStatesMapArray m_effectStatesMapArray;
 
     DISALLOW_COPY_AND_ASSIGN(EffectChain);
 };
diff --git a/src/engine/effects/message.h b/src/engine/effects/message.h
index 12108f5f34..22a5b2f014 100644
--- a/src/engine/effects/message.h
+++ b/src/engine/effects/message.h
@@ -71,6 +71,18 @@ struct EffectsRequest {
 #undef CLEAR_STRUCT
     }
 
+    ~EffectsRequest() {
+        if (type == ENABLE_EFFECT_CHAIN_FOR_INPUT_CHANNEL) {
+            VERIFY_OR_DEBUG_ASSERT(EnableInputChannelForChain.pEffectStatesMapArray != nullptr) {
+                return;
+            }
+            // This only deletes the container used to passed the EffectStates
+            // to EffectProcessorImpl. The EffectStates are managed by
+            // EffectProcessorImpl.
+            delete EnableInputChannelForChain.pEffectStatesMapArray;
+        }
+    }
+
     MessageType type;
     qint64 request_id;