From 74dc8225d1aabebf0c8d42a8559c8d25851f05e5 Mon Sep 17 00:00:00 2001
From: Robbert van der Helm <mail@robbertvanderhelm.nl>
Date: Thu, 21 Jan 2021 01:51:21 +0100
Subject: [PATCH] Back the VST3 plugin factory by an IPtr

This prevents REAPER from crashing when removing the last instance of a
plugin and then readding it. REAPER doesn't unload the module even after
it removes its last plugin factory instance. This means that before this
the plugin factory would be freed but we still had a seemingly valid
pointer to it that we would try to access.
---
 .../bridges/vst3-impls/plugin-factory.cpp     |  1 +
 .../bridges/vst3-impls/plugin-factory.h       |  6 ++++-
 src/plugin/bridges/vst3.cpp                   | 23 ++++++++++---------
 src/plugin/bridges/vst3.h                     | 12 ++++------
 4 files changed, 22 insertions(+), 20 deletions(-)

diff --git a/src/plugin/bridges/vst3-impls/plugin-factory.cpp b/src/plugin/bridges/vst3-impls/plugin-factory.cpp
index 3411d0a7..7e92d942 100644
--- a/src/plugin/bridges/vst3-impls/plugin-factory.cpp
+++ b/src/plugin/bridges/vst3-impls/plugin-factory.cpp
@@ -18,6 +18,7 @@
 
 #include <pluginterfaces/vst/ivstcomponent.h>
 
+#include "../vst3.h"
 #include "plugin-proxy.h"
 
 YaPluginFactoryImpl::YaPluginFactoryImpl(Vst3PluginBridge& bridge,
diff --git a/src/plugin/bridges/vst3-impls/plugin-factory.h b/src/plugin/bridges/vst3-impls/plugin-factory.h
index f0e8ac7e..86c84bfe 100644
--- a/src/plugin/bridges/vst3-impls/plugin-factory.h
+++ b/src/plugin/bridges/vst3-impls/plugin-factory.h
@@ -16,7 +16,11 @@
 
 #pragma once
 
-#include "../vst3.h"
+#include "../../../common/serialization/vst3/plugin-factory.h"
+
+// We need an `IPtr<YaPluginFactoryImpl>` in `Vst3PluginBridge`, so we need to
+// declare this slightly differently to avoid circular includes.
+class Vst3PluginBridge;
 
 class YaPluginFactoryImpl : public YaPluginFactory {
    public:
diff --git a/src/plugin/bridges/vst3.cpp b/src/plugin/bridges/vst3.cpp
index 67834146..d8e3f2f3 100644
--- a/src/plugin/bridges/vst3.cpp
+++ b/src/plugin/bridges/vst3.cpp
@@ -351,15 +351,12 @@ Vst3PluginBridge::~Vst3PluginBridge() {
 }
 
 Steinberg::IPluginFactory* Vst3PluginBridge::get_plugin_factory() {
-    // Even though we're working with raw pointers here, we should pretend that
-    // we're `IPtr<Steinberg::IPluginFactory>` and do the reference counting
-    // ourselves. This should work the same was as the standard implementation
-    // in `public.sdk/source/main/pluginfactory.h`. If we were to use an IPtr or
-    // an STL smart pointer we would get a double free (or rather, a use after
-    // free).
-    if (plugin_factory) {
-        plugin_factory->addRef();
-    } else {
+    // This works the same way as the default implementation in
+    // `public.sdk/source/main/pluginfactory.h`, with the exception that we back
+    // the plugin factory with an `IPtr` ourselves so it cannot be freed before
+    // `Vst3PluginBridge` gets freed. This is needed for REAPER as REAPER does
+    // not call `ModuleExit()`.
+    if (!plugin_factory) {
         // Set up the plugin factory, since this is the first thing the host
         // will request after loading the module. Host callback handlers should
         // have started before this since the Wine plugin host will request a
@@ -368,10 +365,14 @@ Steinberg::IPluginFactory* Vst3PluginBridge::get_plugin_factory() {
             sockets.host_vst_control.send_message(
                 YaPluginFactory::Construct{},
                 std::pair<Vst3Logger&, bool>(logger, true));
-        plugin_factory =
-            new YaPluginFactoryImpl(*this, std::move(factory_args));
+        plugin_factory = Steinberg::owned(
+            new YaPluginFactoryImpl(*this, std::move(factory_args)));
     }
 
+    // Because we're returning a raw pointer, we have to increas the
+    // reference count ourselves
+    plugin_factory->addRef();
+
     return plugin_factory;
 }
 
diff --git a/src/plugin/bridges/vst3.h b/src/plugin/bridges/vst3.h
index e5a2d31d..b4841ee6 100644
--- a/src/plugin/bridges/vst3.h
+++ b/src/plugin/bridges/vst3.h
@@ -22,10 +22,8 @@
 #include "../../common/communication/vst3.h"
 #include "../../common/logging/vst3.h"
 #include "common.h"
-
-// Forward declarations
-class Vst3PluginProxyImpl;
-class YaPluginFactoryImpl;
+#include "vst3-impls/plugin-factory.h"
+#include "vst3-impls/plugin-proxy.h"
 
 /**
  * This handles the communication between the native host and a VST3 plugin
@@ -139,13 +137,11 @@ class Vst3PluginBridge : PluginBridge<Vst3Sockets<std::jthread>> {
      * Our plugin factory. All information about the plugin and its supported
      * classes are copied directly from the Windows VST3 plugin's factory on the
      * Wine side, and we'll provide an implementation that can send control
-     * messages to the Wine plugin host. As explained in `get_plugin_factory()`,
-     * this cannot be a smart pointer because the factory is supposed to free
-     * itself when the host removes its last adopted `IPtr<IpluginFactory>`.
+     * messages to the Wine plugin host.
      *
      * @related get_plugin_factory
      */
-    YaPluginFactoryImpl* plugin_factory = nullptr;
+    Steinberg::IPtr<YaPluginFactoryImpl> plugin_factory = nullptr;
 
    private:
     /**