use atomic access for shared_ptr

daschuer · Dec 4, 2024 · ee27cf7 · ee27cf7
1 parent 36df396
commit ee27cf7
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 11 deletions.
diff --git a/src/controllers/bulk/bulkcontroller.cpp b/src/controllers/bulk/bulkcontroller.cpp
@@ -267,8 +267,8 @@ void BulkController::send(const QList<int>& data, unsigned int length) {
 
 bool BulkController::sendBytes(const QByteArray& data) {
     VERIFY_OR_DEBUG_ASSERT(!m_pMapping ||
-            m_pMapping->getDeviceDirection() &
-                    LegacyControllerMapping::DeviceDirection::Outgoing) {
+            (m_pMapping->getDeviceDirection() &
+                    LegacyControllerMapping::DeviceDirection::Outgoing)) {
         qDebug() << "The mapping for the bulk device" << getName()
                  << "doesn't require sending data. Ignoring sending request.";
         return false;

diff --git a/src/controllers/midi/midicontroller.cpp b/src/controllers/midi/midicontroller.cpp
@@ -2,6 +2,7 @@
 
 #include <QJSValue>
 #include <algorithm>
+#include <atomic>
 
 #include "control/controlobject.h"
 #include "control/controlpotmeter.h"
@@ -38,7 +39,7 @@ MidiController::MidiController(const QString& deviceName)
 
 void MidiController::slotBeforeEngineShutdown() {
     Controller::slotBeforeEngineShutdown();
-    m_pMapping->removeInputHandlerMappings();
+    getSharedMapping()->removeInputHandlerMappings();
 }
 
 MidiController::~MidiController() {
@@ -56,12 +57,21 @@ QString MidiController::mappingExtension() {
 }
 
 void MidiController::setMapping(std::shared_ptr<LegacyControllerMapping> pMapping) {
-    m_pMapping = downcastAndTakeOwnership<LegacyMidiControllerMapping>(std::move(pMapping));
+    std::shared_ptr<LegacyMidiControllerMapping> pMidiMapping =
+            downcastAndTakeOwnership<LegacyMidiControllerMapping>(
+                    std::move(pMapping));
+#ifdef __cpp_lib_atomic_shared_ptr
+    // Used Spinlock
+    m_pMapping.store(pMidiMapping, std::memory_order_relaxed);
+#else
+    // Uses Mutex
+    std::atomic_store_explicit(&m_pMapping, pMidiMapping, std::memory_order_relaxed);
+#endif
 }
 
 std::shared_ptr<LegacyControllerMapping> MidiController::cloneMapping() {
     // Function becomes temporary shared owner
-    std::shared_ptr<LegacyMidiControllerMapping> pMapping = m_pMapping;
+    std::shared_ptr<LegacyMidiControllerMapping> pMapping = getSharedMapping();
     if (!pMapping) {
         return nullptr;
     }
@@ -96,7 +106,7 @@ bool MidiController::applyMapping(const QString& resourcePath) {
 
 void MidiController::createOutputHandlers() {
     // Function becomes temporary shared owner
-    std::shared_ptr<LegacyMidiControllerMapping> pMapping = m_pMapping;
+    std::shared_ptr<LegacyMidiControllerMapping> pMapping = getSharedMapping();
     if (!pMapping) {
         return;
     }
@@ -229,7 +239,7 @@ void MidiController::clearTemporaryInputMappings() {
 
 void MidiController::commitTemporaryInputMappings() {
     // Function becomes temporary shared owner
-    std::shared_ptr<LegacyMidiControllerMapping> pMapping = m_pMapping;
+    std::shared_ptr<LegacyMidiControllerMapping> pMapping = getSharedMapping();
     if (!pMapping) {
         return;
     }
@@ -294,7 +304,7 @@ void MidiController::receivedShortMessage(unsigned char status,
     }
 
     // Function becomes temporary shared owner
-    std::shared_ptr<LegacyMidiControllerMapping> pMapping = m_pMapping;
+    std::shared_ptr<LegacyMidiControllerMapping> pMapping = getSharedMapping();
     auto it = pMapping->getInputMappings().constFind(mappingKey.key);
     for (; it != pMapping->getInputMappings().constEnd() && it.key() == mappingKey.key; ++it) {
         processInputMapping(it.value(), status, control, value, timestamp);
@@ -598,7 +608,7 @@ void MidiController::receive(const QByteArray& data, mixxx::Duration timestamp)
     }
 
     // Function becomes temporary shared owner
-    std::shared_ptr<LegacyMidiControllerMapping> pMapping = m_pMapping;
+    std::shared_ptr<LegacyMidiControllerMapping> pMapping = getSharedMapping();
     const auto [inputMappingsBegin, inputMappingsEnd] =
             pMapping->getInputMappings().equal_range(mappingKey.key);
     std::for_each(inputMappingsBegin, inputMappingsEnd, [&](const auto& inputMapping) {
@@ -651,7 +661,7 @@ QJSValue MidiController::makeInputHandler(int status, int midino, const QJSValue
     const auto midiKey = MidiKey(status, midino);
 
     // Function becomes temporary shared owner
-    std::shared_ptr<LegacyMidiControllerMapping> pMapping = m_pMapping;
+    std::shared_ptr<LegacyMidiControllerMapping> pMapping = getSharedMapping();
     auto it = pMapping->getInputMappings().constFind(midiKey.key);
     if (it != pMapping->getInputMappings().constEnd() &&
             it.value().options.testFlag(MidiOption::Script) &&
@@ -672,3 +682,13 @@ QJSValue MidiController::makeInputHandler(int status, int midino, const QJSValue
     pMapping->addInputMapping(inputMapping.key.key, inputMapping);
     return pJsEngine->newQObject(new MidiInputHandleJSProxy(pMapping, inputMapping));
 }
+
+std::shared_ptr<LegacyMidiControllerMapping> MidiController::getSharedMapping() const {
+#ifdef __cpp_lib_atomic_shared_ptr
+    // Used Spinlock
+    return m_pMapping.load(std::memory_order_relaxed);
+#else
+    // Uses Mutex
+    return std::atomic_load_explicit(&m_pMapping, std::memory_order_relaxed);
+#endif
+}
diff --git a/src/controllers/midi/midicontroller.h b/src/controllers/midi/midicontroller.h
@@ -42,7 +42,7 @@ class MidiController : public Controller {
     virtual std::shared_ptr<LegacyControllerMapping> cloneMapping() override;
 
     bool isMappable() const override {
-        std::shared_ptr<LegacyMidiControllerMapping> pMapping = m_pMapping;
+        std::shared_ptr<LegacyMidiControllerMapping> pMapping = getSharedMapping();
         if (!pMapping) {
             return false;
         }
@@ -103,10 +103,17 @@ class MidiController : public Controller {
     void createOutputHandlers();
     void updateAllOutputs();
     void destroyOutputHandlers();
+    std::shared_ptr<LegacyMidiControllerMapping> getSharedMapping() const;
 
     QHash<uint16_t, MidiInputMapping> m_temporaryInputMappings;
     QList<MidiOutputHandler*> m_outputs;
+
+#ifdef __cpp_lib_atomic_shared_ptr
+    std::atomic<std::shared_ptr<LegacyMidiControllerMapping>> m_pMapping;
+#else
     std::shared_ptr<LegacyMidiControllerMapping> m_pMapping;
+#endif
+
     SoftTakeoverCtrl m_st;
     QList<QPair<MidiInputMapping, unsigned char>> m_fourteen_bit_queued_mappings;