Add more thread-safety in VST

vst/SfizzVstController.cpp

@@ -140,16 +140,20 @@ IPlugView* PLUGIN_API SfizzVstController::createView(FIDString _name)
         return nullptr;
 
     if (_editor) {
+        withStateLock([this]() {
             _uiState = _editor->getCurrentUiState();
+        });
         _editor.reset();
     }
 
     SfizzVstEditor* editor = new SfizzVstEditor(this);
     _editor = Steinberg::owned(editor);
 
+    withStateLock([this, editor]() {
         editor->updateState(_state);
         editor->updateUiState(_uiState);
         editor->updatePlayState(_playState);
+    });
 
     editor->remember();
     return editor;
@@ -204,53 +208,61 @@ tresult PLUGIN_API SfizzVstController::setParamNormalized(Vst::ParamID tag, Vst:
     }
 
     if (slotF32 && *slotF32 != value) {
+        withStateLock([this, slotF32, value]() {
             *slotF32 = value;
             if (SfizzVstEditor* editor = _editor)
                 editor->updateState(_state);
+        });
     }
     else if (slotI32 && *slotI32 != (int32)value) {
+        withStateLock([this, slotI32, value]() {
             *slotI32 = (int32)value;
             if (SfizzVstEditor* editor = _editor)
                 editor->updateState(_state);
+        });
     }
 
     return kResultTrue;
 }
 
-tresult PLUGIN_API SfizzVstController::setState(IBStream* state)
+tresult PLUGIN_API SfizzVstController::setState(IBStream* stream)
 {
     SfizzUiState s;
 
-    tresult r = s.load(state);
+    tresult r = s.load(stream);
     if (r != kResultTrue)
         return r;
 
+    withStateLock([this, &s]() {
         _uiState = s;
-
         if (SfizzVstEditor* editor = _editor)
             editor->updateUiState(_uiState);
+    });
 
     return kResultTrue;
 }
 
-tresult PLUGIN_API SfizzVstController::getState(IBStream* state)
+tresult PLUGIN_API SfizzVstController::getState(IBStream* stream)
 {
+    tresult result;
+
+    withStateLock([this, stream, &result]() {
         if (_editor)
             _uiState = _editor->getCurrentUiState();
+        result = _uiState.store(stream);
+    });
 
-    return _uiState.store(state);
+    return result;
 }
 
-tresult PLUGIN_API SfizzVstController::setComponentState(IBStream* state)
+tresult PLUGIN_API SfizzVstController::setComponentState(IBStream* stream)
 {
     SfizzVstState s;
 
-    tresult r = s.load(state);
+    tresult r = s.load(stream);
     if (r != kResultTrue)
         return r;
 
-    _state = s;
-
     setParamNormalized(kPidVolume, kParamVolumeRange.normalize(s.volume));
     setParamNormalized(kPidNumVoices, kParamNumVoicesRange.normalize(s.numVoices));
     setParamNormalized(kPidOversampling, kParamOversamplingRange.normalize(s.oversamplingLog2));
@@ -259,14 +271,19 @@ tresult PLUGIN_API SfizzVstController::setComponentState(IBStream* state)
     setParamNormalized(kPidTuningFrequency, kParamTuningFrequencyRange.normalize(s.tuningFrequency));
     setParamNormalized(kPidStretchedTuning, kParamStretchedTuningRange.normalize(s.stretchedTuning));
 
+    withStateLock([this, &s]() {
+        _state = s;
         if (SfizzVstEditor* editor = _editor)
             editor->updateState(_state);
+    });
 
     return kResultTrue;
 }
 
 tresult SfizzVstController::notify(Vst::IMessage* message)
 {
+    // Note: may be called from any thread (Reaper)
+
     tresult result = SfizzVstControllerNoUi::notify(message);
     if (result != kResultFalse)
         return result;
@@ -282,10 +299,11 @@ tresult SfizzVstController::notify(Vst::IMessage* message)
         if (result != kResultTrue)
             return result;
 
+        withStateLock([this, data, size]() {
             _state.sfzFile.assign(static_cast<const char *>(data), size);
-
             if (SfizzVstEditor* editor = _editor)
                 editor->updateState(_state);
+        });
     }
     else if (!strcmp(id, "LoadedScala")) {
         const void* data = nullptr;
@@ -295,10 +313,11 @@ tresult SfizzVstController::notify(Vst::IMessage* message)
         if (result != kResultTrue)
             return result;
 
+        withStateLock([this, data, size]() {
             _state.scalaFile.assign(static_cast<const char *>(data), size);
-
             if (SfizzVstEditor* editor = _editor)
                 editor->updateState(_state);
+        });
     }
     else if (!strcmp(id, "NotifiedPlayState")) {
         const void* data = nullptr;
@@ -308,10 +327,11 @@ tresult SfizzVstController::notify(Vst::IMessage* message)
         if (result != kResultTrue)
             return result;
 
+        withStateLock([this, data]() {
             _playState = *static_cast<const SfizzPlayState*>(data);
-
             if (SfizzVstEditor* editor = _editor)
                 editor->updatePlayState(_playState);
+        });
     }
     else if (!strcmp(id, "ReceivedMessage")) {
         const void* data = nullptr;

vst/SfizzVstController.h

@@ -10,6 +10,7 @@
 #include "public.sdk/source/vst/vstparameters.h"
 #include "vstgui/plugin-bindings/vst3editor.h"
 #include <sfizz_message.h>
+#include <mutex>
 class SfizzVstState;
 class SfizzVstEditor;
 
@@ -43,9 +44,9 @@ public:
     IPlugView* PLUGIN_API createView(FIDString name) override;
 
     tresult PLUGIN_API setParamNormalized(Vst::ParamID tag, Vst::ParamValue value) override;
-    tresult PLUGIN_API setState(IBStream* state) override;
+    tresult PLUGIN_API setState(IBStream* stream) override;
-    tresult PLUGIN_API getState(IBStream* state) override;
+    tresult PLUGIN_API getState(IBStream* stream) override;
-    tresult PLUGIN_API setComponentState(IBStream* state) override;
+    tresult PLUGIN_API setComponentState(IBStream* stream) override;
     tresult PLUGIN_API notify(Vst::IMessage* message) override;
 
     ///
@@ -54,8 +55,16 @@ public:
     static FUID cid;
 
 private:
-    SfizzVstState _state;
+    template <class F> void withStateLock(F&& fn) const
-    SfizzUiState _uiState;
+    {
+        std::lock_guard<std::mutex> lock(_stateMutex);
+        fn();
+    }
+
+private:
+    mutable std::mutex _stateMutex; // for R/W the state data
+    SfizzVstState _state {};
+    SfizzUiState _uiState {}; // updated on UI open/close/state-request
     SfizzPlayState _playState {};
     Steinberg::IPtr<SfizzVstEditor> _editor;
 };

vst/SfizzVstEditor.cpp

@@ -25,7 +25,6 @@ SfizzVstEditor::SfizzVstEditor(SfizzVstController* controller)
     : VSTGUIEditor(controller, &sfizzUiViewRect),
       oscTemp_(new uint8_t[kOscTempSize])
 {
-    oscQueue_.reserve(kOscQueueSize);
 }
 
 SfizzVstEditor::~SfizzVstEditor()
@@ -56,10 +55,14 @@ bool PLUGIN_API SfizzVstEditor::open(void* parent, const VSTGUI::PlatformType& p
         editor_.reset(editor);
     }
 
+    withStateLock([this]() {
         mustRedisplayState_ = true;
         mustRedisplayUiState_ = true;
         mustRedisplayPlayState_ = true;
-    flushOscQueue();
+        OscByteVec* queue = new OscByteVec;
+        oscQueue_.reset(queue);
+        queue->reserve(kOscQueueSize);
+    });
 
     updateStateDisplay();
 
@@ -86,7 +89,9 @@ void PLUGIN_API SfizzVstEditor::close()
         this->frame = nullptr;
     }
 
-    flushOscQueue();
+    withStateLock([this]() {
+        oscQueue_.reset();
+    });
 }
 
 ///
@@ -121,51 +126,58 @@ CMessageResult SfizzVstEditor::notify(CBaseObject* sender, const char* message)
 
 void SfizzVstEditor::updateState(const SfizzVstState& state)
 {
-    std::lock_guard<std::recursive_mutex> lock(stateMutex_);
+    withStateLock([this, &state]() {
         state_ = state;
         mustRedisplayState_ = true;
+    });
 }
 
 void SfizzVstEditor::updateUiState(const SfizzUiState& uiState)
 {
-    std::lock_guard<std::recursive_mutex> lock(stateMutex_);
+    withStateLock([this, &uiState]() {
         uiState_ = uiState;
         mustRedisplayUiState_ = true;
+    });
 }
 
 void SfizzVstEditor::updatePlayState(const SfizzPlayState& playState)
 {
-    std::lock_guard<std::recursive_mutex> lock(stateMutex_);
+    withStateLock([this, &playState]() {
         playState_ = playState;
         mustRedisplayPlayState_ = true;
+    });
 }
 
 SfizzUiState SfizzVstEditor::getCurrentUiState() const
 {
-    std::lock_guard<std::recursive_mutex> lock(stateMutex_);
+    SfizzUiState uiState;
-    return uiState_;
+    withStateLock([this, &uiState]() {
+        uiState = uiState_;
+    });
+    return uiState;
 }
 
 void SfizzVstEditor::receiveMessage(const void* data, uint32_t size)
 {
-    if (!frame) {
+    // Note: may be called from non-UI thread (Reaper)
-        // only accumulate if message processing is active
-        return;
-    }
 
-    std::lock_guard<std::recursive_mutex> lock(stateMutex_);
+    withStateLock([this, data, size]() {
-    std::copy(
+        if (OscByteVec* queue = oscQueue_.get()) {
-        reinterpret_cast<const uint8_t*>(data),
+            const uint8_t* bytes = reinterpret_cast<const uint8_t*>(data);
-        reinterpret_cast<const uint8_t*>(data) + size,
+            std::copy(bytes, bytes + size, std::back_inserter(*queue));
-        std::back_inserter(oscQueue_));
+        }
+    });
 }
 
 void SfizzVstEditor::processOscQueue()
 {
-    std::lock_guard<std::recursive_mutex> lock(stateMutex_);
+    withStateLock([this]() {
+        OscByteVec* queue = oscQueue_.get();
+        if (!queue)
+            return;
 
-    const uint8_t* oscData = oscQueue_.data();
+        const uint8_t* oscData = queue->data();
-    size_t oscSize = oscQueue_.size();
+        size_t oscSize = queue->size();
 
         const char* path;
         const char* sig;
@@ -179,13 +191,8 @@ void SfizzVstEditor::processOscQueue()
             oscSize -= msgSize;
         }
 
-    oscQueue_.clear();
+        queue->clear();
-}
+    });
-
-void SfizzVstEditor::flushOscQueue()
-{
-    std::lock_guard<std::recursive_mutex> lock(stateMutex_);
-    oscQueue_.clear();
 }
 
 ///
@@ -333,12 +340,7 @@ void SfizzVstEditor::updateStateDisplay()
     if (!frame)
         return;
 
-    if (!(mustRedisplayState_ || mustRedisplayUiState_ || mustRedisplayPlayState_))
+    withStateLock([this]() {
-        return;
-
-    std::lock_guard<std::recursive_mutex> lock(stateMutex_);
-
-    ///
         if (mustRedisplayState_) {
             uiReceiveValue(EditId::SfzFile, state_.sfzFile);
             uiReceiveValue(EditId::Volume, state_.volume);
@@ -368,6 +370,7 @@ void SfizzVstEditor::updateStateDisplay()
             uiReceiveValue(EditId::UINumActiveVoices, playState_.activeVoices);
             mustRedisplayPlayState_ = false;
         }
+    });
 }
 
 Vst::ParamID SfizzVstEditor::parameterOfEditId(EditId id)

vst/SfizzVstEditor.h

@@ -43,7 +43,12 @@ public:
 
 private:
     void processOscQueue();
-    void flushOscQueue();
+
+    template <class F> void withStateLock(F&& fn) const
+    {
+        std::lock_guard<std::recursive_mutex> lock(stateMutex_);
+        fn();
+    }
 
 protected:
     // EditorController
@@ -72,12 +77,13 @@ private:
 
     // editor state
     // note: might be updated from a non-UI thread
-    mutable std::recursive_mutex stateMutex_;
+    mutable std::recursive_mutex stateMutex_;  // for R/W the state data, and OSC queue
-    SfizzVstState state_;
+    SfizzVstState state_ {};
-    SfizzUiState uiState_;
+    SfizzUiState uiState_ {};
-    SfizzPlayState playState_;
+    SfizzPlayState playState_ {};
     volatile bool mustRedisplayState_ = false;
     volatile bool mustRedisplayUiState_ = false;
     volatile bool mustRedisplayPlayState_ = false;
-    std::vector<uint8_t> oscQueue_;
+    typedef std::vector<uint8_t> OscByteVec;
+    std::unique_ptr<OscByteVec> oscQueue_;
 };