From c8dae58fd211ee364f641787e97bf7f279a914ef Mon Sep 17 00:00:00 2001
From: reuk <reuk@users.noreply.github.com>
Date: Fri, 7 May 2021 18:35:06 +0100
Subject: [PATCH] VST3 Host:  Avoid calling processBlock, prepareToPlay, and
 releaseResources simultaneously

The old design had us checking isActive inside our audio callbacks, and
also modifying isActive from prepareToPlay(), and releaseResources(). To
avoid race conditions, and to ensure that isActive actually reflects the
activation state of our plugin, we need to lock in these places. If we
don't lock, there's a chance that other threads will observe isActive to
be true while the plugin is not actually active (for example).

If you're not convinced, imagine this scenario:
- The message thread calls prepareToPlay. The plugin is activated.
- The audio thread starts calling processBlock, and gets as far as the
  isActive check. The plugin appears active, so we continue into
  processBlock.
- At the same time, the message thread calls releaseResources(). The
  processBlock call is still in progress, but the message thread is
  simultaneously making calls to setProcessing() and setActive(), which
  is a race.

Normally, it'd be up to the host of the AudioProcessor to ensure that
prepareToPlay() isn't called at the same time as processBlock().
However, VST3 plugins can request a restart at any time from the UI
thread, requiring us to call prepareToPlay() even while processBlock()
is running.
---
 .../format_types/juce_VST3PluginFormat.cpp    | 28 ++++++++++++++++++-
 1 file changed, 27 insertions(+), 1 deletion(-)

diff --git a/modules/juce_audio_processors/format_types/juce_VST3PluginFormat.cpp b/modules/juce_audio_processors/format_types/juce_VST3PluginFormat.cpp
index e60e327f22..48b3c674b7 100644
--- a/modules/juce_audio_processors/format_types/juce_VST3PluginFormat.cpp
+++ b/modules/juce_audio_processors/format_types/juce_VST3PluginFormat.cpp
@@ -2294,6 +2294,8 @@ public:
         JUCE_ASSERT_MESSAGE_THREAD
         MessageManagerLock lock;
 
+        const SpinLock::ScopedLockType processLock (processMutex);
+
         // Avoid redundantly calling things like setActive, which can be a heavy-duty call for some plugins:
         if (isActive
               && getSampleRate() == newSampleRate
@@ -2353,6 +2355,8 @@ public:
 
     void releaseResources() override
     {
+        const SpinLock::ScopedLockType lock (processMutex);
+
         if (! isActive)
             return; // Avoids redundantly calling things like setActive
 
@@ -2392,6 +2396,8 @@ public:
     {
         jassert (! isUsingDoublePrecision());
 
+        const SpinLock::ScopedLockType processLock (processMutex);
+
         if (isActive && processor != nullptr)
             processAudio (buffer, midiMessages, Vst::kSample32, false);
     }
@@ -2400,6 +2406,8 @@ public:
     {
         jassert (isUsingDoublePrecision());
 
+        const SpinLock::ScopedLockType processLock (processMutex);
+
         if (isActive && processor != nullptr)
             processAudio (buffer, midiMessages, Vst::kSample64, false);
     }
@@ -2408,6 +2416,8 @@ public:
     {
         jassert (! isUsingDoublePrecision());
 
+        const SpinLock::ScopedLockType processLock (processMutex);
+
         if (bypassParam != nullptr)
         {
             if (isActive && processor != nullptr)
@@ -2423,6 +2433,8 @@ public:
     {
         jassert (isUsingDoublePrecision());
 
+        const SpinLock::ScopedLockType processLock (processMutex);
+
         if (bypassParam != nullptr)
         {
             if (isActive && processor != nullptr)
@@ -2489,6 +2501,8 @@ public:
 
     bool isBusesLayoutSupported (const BusesLayout& layouts) const override
     {
+        const SpinLock::ScopedLockType processLock (processMutex);
+
         // if the processor is not active, we ask the underlying plug-in if the
         // layout is actually supported
         if (! isActive)
@@ -2743,6 +2757,8 @@ public:
     //==============================================================================
     void reset() override
     {
+        const SpinLock::ScopedLockType lock (processMutex);
+
         if (holder->component != nullptr && processor != nullptr)
         {
             processor->setProcessing (false);
@@ -2897,6 +2913,14 @@ private:
     std::map<Vst::ParamID, VST3Parameter*> idToParamMap;
     EditControllerParameterDispatcher parameterDispatcher;
 
+    /*  The plugin may request a restart during playback, which may in turn
+        attempt to call functions such as setProcessing and setActive. It is an
+        error to call these functions simultaneously with
+        IAudioProcessor::process, so we use this mutex to ensure that this
+        scenario is impossible.
+    */
+    SpinLock processMutex;
+
     //==============================================================================
     template <typename Type>
     static void appendStateFrom (XmlElement& head, VSTComSmartPtr<Type>& object, const String& identifier)
@@ -3210,7 +3234,7 @@ private:
     {
         Vst::ParameterInfo paramInfo{};
 
-        if (processor != nullptr)
+        if (editController != nullptr)
             editController->getParameterInfo ((int32) index, paramInfo);
 
         return paramInfo;
@@ -3380,6 +3404,8 @@ tresult VST3HostContext::restartComponent (Steinberg::int32 flags)
             auto sampleRate = plugin->getSampleRate();
             auto blockSize  = plugin->getBlockSize();
 
+            // Have to deactivate here, otherwise prepareToPlay might not pick up the new bus layouts
+            plugin->releaseResources();
             plugin->prepareToPlay (sampleRate >= 8000 ? sampleRate : 44100.0,
                                    blockSize > 0 ? blockSize : 1024);
         }