Housekeeping: Add Some Memory Safety Clamps

include/element/processor.hpp

@@ -228,6 +228,14 @@ class Processor : public juce::ReferenceCountedObject {
     /** Suspend processing */
     void suspendProcessing (const bool);
 
+    /** Upper bound applied to any latency value in samples.
+
+        Plugin-reported latency and delay compensation size per-channel delay
+        buffers in the graph builder, so a bogus value would otherwise allocate
+        gigabytes. About 21 seconds at 48 kHz.
+    */
+    static constexpr int maxLatencySamples = 1 << 20;
+
     /** Get latency audio samples */
     int getLatencySamples() const;
 
@@ -516,7 +524,7 @@ class Processor : public juce::ReferenceCountedObject {
     }
 
     //==========================================================================
-    /** Set latency samples */
+    /** Set latency samples. Values are clamped to [0, maxLatencySamples]. */
     void setLatencySamples (int latency);
 
     //==========================================================================
@@ -621,6 +629,7 @@ class Processor : public juce::ReferenceCountedObject {
 
     double delayCompMillis = 0.0;
     int delayCompSamples = 0;
+    void updateDelayCompensationSamples();
 
     juce::AudioPlayHead* _playhead { nullptr };
 

src/engine/graphnode.cpp

@@ -457,7 +457,7 @@ void GraphNode::buildRenderingSequence()
         // swap over to the new rendering sequence..
         {
             const ScopedLock sl (getPropertyLock());
-            renderingBuffers.setSize (numRenderingBuffersNeeded, 4096);
+            renderingBuffers.setSize (numRenderingBuffersNeeded, jmax (4096, getBlockSize()));
             renderingBuffers.clear();
 
             for (int i = midiBuffers.size(); --i >= 0;)

src/engine/processor.cpp

@@ -337,6 +337,7 @@ void Processor::prepare (const double newSampleRate,
     sampleRate = newSampleRate;
     blockSize = newBlockSize;
     parent = parentGraph;
+    updateDelayCompensationSamples();
 
     if ((willBeEnabled || enabled.get() == 1) && ! isPrepared)
     {
@@ -891,13 +892,29 @@ int Processor::getOversamplingFactor()
 }
 
 //==============================================================================
+static int clampLatencySamples (const Processor& proc, int samples, const char* what)
+{
+    if (samples >= 0 && samples <= Processor::maxLatencySamples)
+        return samples;
+    Logger::writeToLog (String ("[element] ") + proc.getName() + ": clamping " + what
+                        + " of " + String (samples) + " samples");
+    return jlimit (0, Processor::maxLatencySamples, samples);
+}
+
 void Processor::setDelayCompensation (double delayMs)
 {
-    if (delayCompMillis == delayMs)
-        return;
     delayCompMillis = delayMs;
-    jassert (sampleRate > 0.0);
-    delayCompSamples = roundToInt (delayCompMillis * 0.001 * sampleRate);
+    updateDelayCompensationSamples();
+}
+
+void Processor::updateDelayCompensationSamples()
+{
+    // The sample rate may not be known yet (e.g. state restored before
+    // prepare); prepare() calls this again once it is.
+    if (sampleRate <= 0.0)
+        return;
+    delayCompSamples = clampLatencySamples (
+        *this, roundToInt (delayCompMillis * 0.001 * sampleRate), "delay compensation");
 }
 
 double Processor::getDelayCompensation() const { return delayCompMillis; }
@@ -987,9 +1004,7 @@ int Processor::getLatencySamples() const
 
 void Processor::setLatencySamples (int latency)
 {
-    if (latency == latencySamples)
-        return;
-    latencySamples = latency;
+    latencySamples = clampLatencySamples (*this, latency, "latency");
 }
 
 //=========================================================================

src/engine/rootgraph.cpp

@@ -22,8 +22,8 @@ void RootGraph::setPlayConfigFor (DeviceManager& devices)
 void RootGraph::setPlayConfigFor (AudioIODevice* device)
 {
     jassert (device != nullptr);
-    setRenderDetails (device->getCurrentBufferSizeSamples(),
-                      device->getCurrentSampleRate());
+    setRenderDetails (device->getCurrentSampleRate(),
+                      device->getCurrentBufferSizeSamples());
 }
 
 void RootGraph::setPlayConfigFor (const DeviceManager::AudioDeviceSetup& setup)

src/nodes/audiofileplayer.cpp

@@ -415,6 +415,9 @@ AudioFilePlayerNode::AudioFilePlayerNode()
     addLegacyParameter (volume = new AudioParameterFloat (juce::ParameterID ("volume", 1), "Volume", -60.f, 12.f, 0.f));
     addLegacyParameter (looping = new AudioParameterBool (juce::ParameterID ("loop", 1), "Loop", false));
 
+    // Needed before prepareToPlay: state restore opens the file.
+    formats.registerBasicFormats();
+
     for (auto* const param : getParameters())
         param->addListener (this);
 }
@@ -494,7 +497,6 @@ void AudioFilePlayerNode::openFile (const File& file)
 void AudioFilePlayerNode::prepareToPlay (double sampleRate, int maximumExpectedSamplesPerBlock)
 {
     thread.startThread();
-    formats.registerBasicFormats();
     player.prepareToPlay (maximumExpectedSamplesPerBlock, sampleRate);
 
     if (reader)

src/nodes/mediaplayer.cpp

@@ -163,6 +163,10 @@ MediaPlayerProcessor::MediaPlayerProcessor()
     addLegacyParameter (playing = new AudioParameterBool (juce::ParameterID ("playing", 1), "Playing", false));
     addLegacyParameter (slave = new AudioParameterBool (juce::ParameterID ("slave", 1), "Slave", false));
     addLegacyParameter (volume = new AudioParameterFloat (juce::ParameterID ("volume", 1), "Volume", -60.f, 12.f, 0.f));
+
+    // Needed before prepareToPlay: state restore opens the file.
+    formats.registerBasicFormats();
+
     for (auto* const param : getParameters())
         param->addListener (this);
 }

src/ui/block.cpp

@@ -23,6 +23,12 @@
 
 namespace element {
 
+/** Upper bound on a block's custom size. The block is buffered to an image and
+    has a shadow effect, so an unclamped size read from a session would allocate
+    two enormous bitmaps. */
+static constexpr int maxBlockWidth = 8192;
+static constexpr int maxBlockHeight = 8192;
+
 namespace detail {
 inline static Context* context (juce::Component* comp)
 {
@@ -335,12 +341,17 @@ void BlockComponent::setDisplayModeInternal (DisplayMode mode, bool force)
         {
             struct EmbedBockAsync : MessageManager::MessageBase
             {
-                using PtrType = std::unique_ptr<juce::Component>;
-                EmbedBockAsync (BlockComponent& b, const Node& n, UI& u, PtrType& p, DisplayMode om)
-                    : block (b), node (n), ui (u), embedded (p), oldMode (om) {}
+                EmbedBockAsync (BlockComponent& b, const Node& n, UI& u, DisplayMode om)
+                    : block (&b), node (n), ui (u), oldMode (om) {}
 
                 void messageCallback() override
                 {
+                    // The block may have been deleted (session change, rebuild)
+                    // before this message was delivered.
+                    if (block == nullptr)
+                        return;
+
+                    auto& embedded = block->embedded;
                     ui.closePluginWindowsFor (node, false);
 
                     if (embedded == nullptr)
@@ -356,27 +367,26 @@ void BlockComponent::setDisplayModeInternal (DisplayMode mode, bool force)
 
                     if (embedded != nullptr)
                     {
-                        block.addAndMakeVisible (embedded.get());
-                        block.updateSize();
-                        block.resized();
-                        embedded->addComponentListener (&block);
+                        block->addAndMakeVisible (embedded.get());
+                        block->updateSize();
+                        block->resized();
+                        embedded->addComponentListener (block);
                     }
                     else
                     {
                         if (oldMode != Embed)
-                            block.setDisplayModeInternal (oldMode, true);
+                            block->setDisplayModeInternal (oldMode, true);
                     }
                 }
 
-                BlockComponent& block;
+                Component::SafePointer<BlockComponent> block;
                 Node node;
                 UI& ui;
-                PtrType& embedded;
                 DisplayMode oldMode;
             };
 
             if (auto* ui = ViewHelpers::getGuiController (this))
-                (new EmbedBockAsync (*this, node, *ui, this->embedded, oldMode))->post();
+                (new EmbedBockAsync (*this, node, *ui, oldMode))->post();
         }
         else
         {
@@ -1230,7 +1240,7 @@ void BlockComponent::updateSize()
             {
                 if (detail::canResize (*this) && customWidth > 0 && customHeight > 0)
                 {
-                    setSize (customWidth, customHeight);
+                    setSize (jmin (customWidth, maxBlockWidth), jmin (customHeight, maxBlockHeight));
                     resized();
                 }
                 else
@@ -1254,10 +1264,8 @@ void BlockComponent::setCustomSize (int width, int height)
 {
     int mw = width, mh = height;
     getMinimumSize (mw, mh);
-    if (width < mw)
-        width = mw;
-    if (height < mh)
-        height = mh;
+    width = jlimit (mw, maxBlockWidth, width);
+    height = jlimit (mh, maxBlockHeight, height);
 
     if (customWidth != width || customHeight != height)
     {

src/ui/pluginmanagercomponent.cpp

@@ -582,6 +582,7 @@ PluginListComponent::PluginListComponent (PluginManager& p, PropertiesFile* prop
 
 PluginListComponent::~PluginListComponent()
 {
+    plugins.removeChangeListener (this);
     list.removeChangeListener (this);
 }
 

test/CMakeLists.txt

@@ -70,6 +70,7 @@ add_test(NAME "DSPScriptTest" COMMAND test_element --run_test=DSPScriptTest)
 add_test(NAME "Element" COMMAND test_element --run_test=Element)
 add_test(NAME "GraphNodeTests" COMMAND test_element --run_test=GraphNodeTests)
 add_test(NAME "GzipTests" COMMAND test_element --run_test=GzipTests)
+add_test(NAME "LatencyClampTests" COMMAND test_element --run_test=LatencyClampTests)
 add_test(NAME "IONodeTests" COMMAND test_element --run_test=IONodeTests)
 add_test(NAME "LinearFadeTest" COMMAND test_element --run_test=LinearFadeTest)
 add_test(NAME "MidiChannelMapTest" COMMAND test_element --run_test=MidiChannelMapTest)

test/LatencyClampTests.cpp

@@ -0,0 +1,62 @@
+// SPDX-FileCopyrightText: Copyright (C) Kushview, LLC.
+// SPDX-License-Identifier: GPL-3.0-or-later
+
+#include <boost/test/unit_test.hpp>
+
+#include <element/context.hpp>
+#include <element/processor.hpp>
+
+#include "engine/graphnode.hpp"
+#include "fixture/TestNode.h"
+#include "testutil.hpp"
+
+using namespace element;
+
+namespace {
+struct LatencyNode : public TestNode
+{
+    using TestNode::setLatencySamples;
+};
+} // namespace
+
+BOOST_AUTO_TEST_SUITE (LatencyClampTests)
+
+BOOST_AUTO_TEST_CASE (LatencySamplesAreClamped)
+{
+    LatencyNode node;
+    node.setLatencySamples (512);
+    BOOST_REQUIRE_EQUAL (node.getLatencySamples(), 512);
+
+    node.setLatencySamples (-100);
+    BOOST_REQUIRE_EQUAL (node.getLatencySamples(), 0);
+
+    node.setLatencySamples (std::numeric_limits<int>::max());
+    BOOST_REQUIRE_EQUAL (node.getLatencySamples(), Processor::maxLatencySamples);
+}
+
+BOOST_AUTO_TEST_CASE (DelayCompensationSurvivesRestoreBeforePrepare)
+{
+    // GraphManager-style refcounting means the graph must live on the heap.
+    ProcessorPtr keep (new GraphNode (*element::test::context()));
+    auto& graph = *static_cast<GraphNode*> (keep.get());
+
+    auto* node = graph.addNode (new TestNode (2, 2, 0, 0));
+    BOOST_REQUIRE (node != nullptr);
+
+    // State is restored before the node is prepared during a session load,
+    // so the sample rate is unknown at this point.
+    node->setDelayCompensation (10.0);
+    BOOST_REQUIRE_EQUAL (node->getDelayCompensation(), 10.0);
+    BOOST_REQUIRE_EQUAL (node->getDelayCompensationSamples(), 0);
+
+    graph.prepareToRender (48000.0, 512);
+    BOOST_REQUIRE_EQUAL (node->getDelayCompensationSamples(), 480);
+
+    node->setDelayCompensation (1.0e9);
+    BOOST_REQUIRE_EQUAL (node->getDelayCompensationSamples(), Processor::maxLatencySamples);
+
+    graph.releaseResources();
+    graph.clear();
+}
+
+BOOST_AUTO_TEST_SUITE_END()