Merge pull request #573 from jpcima/weak-ptr

vst: do not keep a strong reference on the editor

vst/CMakeLists.txt

@@ -35,7 +35,8 @@ set(VSTPLUGIN_HEADERS
     SfizzSettings.h
     X11RunLoop.h
     NativeHelpers.h
-    FileTrie.h)
+    FileTrie.h
+    WeakPtr.h)
 
 if(APPLE)
     set(VSTPLUGIN_MAC_SOURCES

vst/SfizzVstController.cpp

@@ -139,15 +139,14 @@ IPlugView* PLUGIN_API SfizzVstController::createView(FIDString _name)
     if (name != Vst::ViewType::kEditor)
         return nullptr;
 
-    if (_editor) {
-        withStateLock([this]() {
-            _uiState = _editor->getCurrentUiState();
+    if (IPtr<SfizzVstEditor> editor = _editor.lock()) {
+        withStateLock([this, editor]() {
+            _uiState = editor->getCurrentUiState();
         });
-        _editor.reset();
     }
 
-    SfizzVstEditor* editor = new SfizzVstEditor(this);
-    _editor = Steinberg::owned(editor);
+    IPtr<SfizzVstEditor> editor = Steinberg::owned(new SfizzVstEditor(this));
+    _editor = editor->getWeakPtr();
 
     withStateLock([this, editor]() {
         editor->updateState(_state);
@@ -210,14 +209,14 @@ tresult PLUGIN_API SfizzVstController::setParamNormalized(Vst::ParamID tag, Vst:
     if (slotF32 && *slotF32 != value) {
         withStateLock([this, slotF32, value]() {
             *slotF32 = value;
-            if (SfizzVstEditor* editor = _editor)
+            if (IPtr<SfizzVstEditor> editor = _editor.lock())
                 editor->updateState(_state);
         });
     }
     else if (slotI32 && *slotI32 != (int32)value) {
         withStateLock([this, slotI32, value]() {
             *slotI32 = (int32)value;
-            if (SfizzVstEditor* editor = _editor)
+            if (IPtr<SfizzVstEditor> editor = _editor.lock())
                 editor->updateState(_state);
         });
     }
@@ -235,7 +234,7 @@ tresult PLUGIN_API SfizzVstController::setState(IBStream* stream)
 
     withStateLock([this, &s]() {
         _uiState = s;
-        if (SfizzVstEditor* editor = _editor)
+        if (IPtr<SfizzVstEditor> editor = _editor.lock())
             editor->updateUiState(_uiState);
     });
 
@@ -247,8 +246,8 @@ tresult PLUGIN_API SfizzVstController::getState(IBStream* stream)
     tresult result;
 
     withStateLock([this, stream, &result]() {
-        if (_editor)
-            _uiState = _editor->getCurrentUiState();
+        if (IPtr<SfizzVstEditor> editor = _editor.lock())
+            _uiState = editor->getCurrentUiState();
         result = _uiState.store(stream);
     });
 
@@ -273,7 +272,7 @@ tresult PLUGIN_API SfizzVstController::setComponentState(IBStream* stream)
 
     withStateLock([this, &s]() {
         _state = s;
-        if (SfizzVstEditor* editor = _editor)
+        if (IPtr<SfizzVstEditor> editor = _editor.lock())
             editor->updateState(_state);
     });
 
@@ -301,7 +300,7 @@ tresult SfizzVstController::notify(Vst::IMessage* message)
 
         withStateLock([this, data, size]() {
             _state.sfzFile.assign(static_cast<const char *>(data), size);
-            if (SfizzVstEditor* editor = _editor)
+            if (IPtr<SfizzVstEditor> editor = _editor.lock())
                 editor->updateState(_state);
         });
     }
@@ -315,7 +314,7 @@ tresult SfizzVstController::notify(Vst::IMessage* message)
 
         withStateLock([this, data, size]() {
             _state.scalaFile.assign(static_cast<const char *>(data), size);
-            if (SfizzVstEditor* editor = _editor)
+            if (IPtr<SfizzVstEditor> editor = _editor.lock())
                 editor->updateState(_state);
         });
     }
@@ -329,7 +328,7 @@ tresult SfizzVstController::notify(Vst::IMessage* message)
 
         withStateLock([this, data]() {
             _playState = *static_cast<const SfizzPlayState*>(data);
-            if (SfizzVstEditor* editor = _editor)
+            if (IPtr<SfizzVstEditor> editor = _editor.lock())
                 editor->updatePlayState(_playState);
         });
     }
@@ -341,7 +340,7 @@ tresult SfizzVstController::notify(Vst::IMessage* message)
         if (result != kResultTrue)
             return result;
 
-        if (SfizzVstEditor* editor = _editor)
+        if (IPtr<SfizzVstEditor> editor = _editor.lock())
             editor->receiveMessage(data, size);
     }
 

vst/SfizzVstController.h

@@ -9,6 +9,7 @@
 #include "public.sdk/source/vst/vsteditcontroller.h"
 #include "public.sdk/source/vst/vstparameters.h"
 #include "vstgui/plugin-bindings/vst3editor.h"
+#include "WeakPtr.h"
 #include <sfizz_message.h>
 #include <mutex>
 class SfizzVstState;
@@ -66,5 +67,5 @@ private:
     SfizzVstState _state {};
     SfizzUiState _uiState {}; // updated on UI open/close/state-request
     SfizzPlayState _playState {};
-    Steinberg::IPtr<SfizzVstEditor> _editor;
+    WeakPtr<SfizzVstEditor> _editor;
 };

vst/SfizzVstEditor.h

@@ -7,6 +7,7 @@
 #pragma once
 #include "SfizzVstController.h"
 #include "editor/EditorController.h"
+#include "WeakPtr.h"
 #include "public.sdk/source/vst/vstguieditor.h"
 #include <mutex>
 class Editor;
@@ -18,8 +19,11 @@ using namespace Steinberg;
 using namespace VSTGUI;
 
 class SfizzVstEditor : public Vst::VSTGUIEditor,
-                       public EditorController {
+                       public EditorController,
+                       public Weakable<SfizzVstEditor> {
 public:
+    using Self = SfizzVstEditor;
+
     explicit SfizzVstEditor(SfizzVstController* controller);
     ~SfizzVstEditor();
 
@@ -41,6 +45,10 @@ public:
     SfizzUiState getCurrentUiState() const;
     void receiveMessage(const void* data, uint32_t size);
 
+    void remember() override { SfizzVstEditor::addRef(); }
+    void forget() override { SfizzVstEditor::release(); }
+    WEAKABLE_REFCOUNT_METHODS(SfizzVstEditor)
+
 private:
     void processOscQueue();
 

vst/WeakPtr.h

@@ -0,0 +1,122 @@
+// SPDX-License-Identifier: BSD-2-Clause
+
+// This code is part of the sfizz library and is licensed under a BSD 2-clause
+// license. You should have receive a LICENSE.md file along with the code.
+// If not, contact the sfizz maintainers at https://github.com/sfztools/sfizz
+
+#pragma once
+#include "base/source/fobject.h"
+#include <memory>
+#include <mutex>
+
+/**
+ * A weak reference implementation for Steinberg FObject.
+ *
+ * Implementation
+ * ==============
+ *
+ * This takes over the ordinary addRef() and release() methods.
+ * The variable `refCount` is accessed manually, under a shared mutex.
+ * There is a unique data block which is shared with all weak pointers, the
+ * system will null it atomically when the reference count hits zero.
+ *
+ * Usage
+ * =====
+ *
+ * class MyObject : public FObject, public Weakable<MyObject> {
+ *   [...]
+ *   WEAKABLE_REFCOUNT_METHODS(MyObject)
+ * };
+ *
+ * WeakPtr<MyObject> ptr = myObject.getWeakPtr();
+ */
+
+template <class T>
+class Weakable;
+
+#define WEAKABLE_REFCOUNT_METHODS(T)                                               \
+public:                                                                            \
+    Steinberg::uint32 PLUGIN_API addRef() SMTG_OVERRIDE { return weakAddRef(); }   \
+    Steinberg::uint32 PLUGIN_API release() SMTG_OVERRIDE { return weakRelease(); } \
+private:                                                                           \
+    friend class Weakable<T>;                                                      \
+    friend class WeakPtr<T>;
+
+///
+template <class T>
+struct WeakPtrSharedData : public std::enable_shared_from_this<WeakPtrSharedData<T>> {
+    explicit WeakPtrSharedData(T* self) : self_(self) {}
+    std::mutex mutex_;
+    T* self_ = nullptr;
+};
+
+///
+template <class T>
+class WeakPtr {
+    friend class Weakable<T>;
+    using SharedData = WeakPtrSharedData<T>;
+
+public:
+    WeakPtr() = default;
+
+    Steinberg::IPtr<T> lock()
+    {
+        std::shared_ptr<SharedData> data = data_.lock();
+        if (!data)
+            return nullptr;
+        std::lock_guard<std::mutex> lock { data->mutex_ };
+        T* self = data->self_;
+        if (self)
+            ++self->refCount; // manually because we are holding the lock
+        return Steinberg::IPtr<T>(self, false);
+    }
+
+private:
+    explicit WeakPtr(std::weak_ptr<SharedData> data) : data_(data) {}
+    std::weak_ptr<SharedData> data_;
+};
+
+///
+template <class T>
+class Weakable {
+    using SharedData = WeakPtrSharedData<T>;
+
+public:
+    Weakable()
+        : weakData_(new SharedData(static_cast<T*>(this)))
+    {
+    }
+
+    WeakPtr<T> getWeakPtr()
+    {
+        return WeakPtr<T>(weakData_);
+    }
+
+protected:
+    Steinberg::uint32 weakAddRef() //override
+    {
+        T* self = static_cast<T*>(this);
+        std::lock_guard<std::mutex> lock { weakData_->mutex_ };
+        return ++self->refCount;
+    }
+
+    Steinberg::uint32 weakRelease() //override
+    {
+        T* self = static_cast<T*>(this);
+        std::shared_ptr<SharedData> data = weakData_;
+        std::unique_lock<std::mutex> lock { data->mutex_ };
+        Steinberg::uint32 count = --self->refCount;
+        if (count == 0) {
+            data->self_ = nullptr;
+            weakData_.reset();
+            self->refCount = -1000;
+            lock.unlock();
+            delete self;
+            return 0;
+        }
+        return count;
+    }
+
+private:
+    std::shared_ptr<SharedData> weakData_;
+};