Hold strong reference when using m_pMapping to avoid race condition.

daschuer · Dec 4, 2024 · 202c657 · 202c657
1 parent 05f60a8
commit 202c657
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 17 deletions.
diff --git a/src/controllers/midi/midicontroller.cpp b/src/controllers/midi/midicontroller.cpp
@@ -59,10 +59,12 @@ void MidiController::setMapping(std::shared_ptr<LegacyControllerMapping> pMappin
 }
 
 std::shared_ptr<LegacyControllerMapping> MidiController::cloneMapping() {
-    if (!m_pMapping) {
+    // Function becomes temporary shared owner
+    std::shared_ptr<LegacyMidiControllerMapping> pMapping = m_pMapping;
+    if (!pMapping) {
         return nullptr;
     }
-    return m_pMapping->clone();
+    return pMapping->clone();
 }
 
 int MidiController::close() {
@@ -92,16 +94,18 @@ bool MidiController::applyMapping() {
 }
 
 void MidiController::createOutputHandlers() {
-    if (!m_pMapping) {
+    // Function becomes temporary shared owner
+    std::shared_ptr<LegacyMidiControllerMapping> pMapping = m_pMapping;
+    if (!pMapping) {
         return;
     }
 
-    if (m_pMapping->getOutputMappings().isEmpty()) {
+    if (pMapping->getOutputMappings().isEmpty()) {
         return;
     }
 
     QStringList failures;
-    for (const auto& mapping : m_pMapping->getOutputMappings()) {
+    for (const auto& mapping : pMapping->getOutputMappings()) {
         QString group = mapping.controlKey.group;
         QString key = mapping.controlKey.item;
 
@@ -223,7 +227,9 @@ void MidiController::clearTemporaryInputMappings() {
 }
 
 void MidiController::commitTemporaryInputMappings() {
-    if (!m_pMapping) {
+    // Function becomes temporary shared owner
+    std::shared_ptr<LegacyMidiControllerMapping> pMapping = m_pMapping;
+    if (!pMapping) {
         return;
     }
 
@@ -232,15 +238,15 @@ void MidiController::commitTemporaryInputMappings() {
     // m_temporaryInputMappings from m_mapping's input mappings.
     for (auto it = m_temporaryInputMappings.constBegin();
          it != m_temporaryInputMappings.constEnd(); ++it) {
-        m_pMapping->removeInputMapping(it.key());
+        pMapping->removeInputMapping(it.key());
     }
 
     // Now, we can just use add all mappings from m_temporaryInputMappings
     // since we removed the duplicates in the original set.
     for (auto it = m_temporaryInputMappings.constBegin();
             it != m_temporaryInputMappings.constEnd();
             ++it) {
-        m_pMapping->addInputMapping(it.key(), it.value());
+        pMapping->addInputMapping(it.key(), it.value());
     }
     m_temporaryInputMappings.clear();
 }
@@ -286,8 +292,10 @@ void MidiController::receivedShortMessage(unsigned char status,
         }
     }
 
-    auto it = m_pMapping->getInputMappings().constFind(mappingKey.key);
-    for (; it != m_pMapping->getInputMappings().constEnd() && it.key() == mappingKey.key; ++it) {
+    // Function becomes temporary shared owner
+    std::shared_ptr<LegacyMidiControllerMapping> pMapping = m_pMapping;
+    auto it = pMapping->getInputMappings().constFind(mappingKey.key);
+    for (; it != pMapping->getInputMappings().constEnd() && it.key() == mappingKey.key; ++it) {
         processInputMapping(it.value(), status, control, value, timestamp);
     }
 }
@@ -584,8 +592,10 @@ void MidiController::receive(const QByteArray& data, mixxx::Duration timestamp)
         }
     }
 
+    // Function becomes temporary shared owner
+    std::shared_ptr<LegacyMidiControllerMapping> pMapping = m_pMapping;
     const auto [inputMappingsBegin, inputMappingsEnd] =
-            m_pMapping->getInputMappings().equal_range(mappingKey.key);
+            pMapping->getInputMappings().equal_range(mappingKey.key);
     std::for_each(inputMappingsBegin, inputMappingsEnd, [&](const auto& inputMapping) {
         processInputMapping(inputMapping, data, timestamp);
     });
@@ -635,8 +645,10 @@ QJSValue MidiController::makeInputHandler(int status, int midino, const QJSValue
 
     const auto midiKey = MidiKey(status, midino);
 
-    auto it = m_pMapping->getInputMappings().constFind(midiKey.key);
-    if (it != m_pMapping->getInputMappings().constEnd() &&
+    // Function becomes temporary shared owner
+    std::shared_ptr<LegacyMidiControllerMapping> pMapping = m_pMapping;
+    auto it = pMapping->getInputMappings().constFind(midiKey.key);
+    if (it != pMapping->getInputMappings().constEnd() &&
             it.value().options.testFlag(MidiOption::Script) &&
             std::holds_alternative<ConfigKey>(it.value().control)) {
         qCWarning(m_logBase) << QStringLiteral(
@@ -652,6 +664,6 @@ QJSValue MidiController::makeInputHandler(int status, int midino, const QJSValue
             MidiOption::Script,
             std::make_shared<QJSValue>(scriptCode));
 
-    m_pMapping->addInputMapping(inputMapping.key.key, inputMapping);
-    return pJsEngine->newQObject(new MidiInputHandleJSProxy(m_pMapping, inputMapping));
+    pMapping->addInputMapping(inputMapping.key.key, inputMapping);
+    return pJsEngine->newQObject(new MidiInputHandleJSProxy(pMapping, inputMapping));
 }
diff --git a/src/controllers/midi/midicontroller.h b/src/controllers/midi/midicontroller.h
@@ -43,10 +43,11 @@ class MidiController : public Controller {
     virtual std::shared_ptr<LegacyControllerMapping> cloneMapping() override;
 
     bool isMappable() const override {
-        if (!m_pMapping) {
+        std::shared_ptr<LegacyMidiControllerMapping> pMapping = m_pMapping;
+        if (!pMapping) {
             return false;
         }
-        return m_pMapping->isMappable();
+        return pMapping->isMappable();
     }
 
     bool matchMapping(const MappingInfo& mapping) override;