Possible pluginval crash bug with AudioProcessor that uses member AudioProcessorGraph internally

Audio Plugins
1

The Initial Problem: Random pluginval Crashes Randomly

I ran into this while testing a plug-in I am developing in pluginval where I kept getting random, inexplicable crashes in pluginval itself with fairly unhelpful stack traces. These crashes would only happen some of the time, making them seem likely related to a threading issue.

Although I was able to pair things down a bit, my plugin was still too complex to easily rule out various issues so I decided to see if I could reproduce the behavior with a new simple plugin project, but at first I could not!

As I did my best to debug with the unhelpful stack traces from pluginval, I noticed that they all seemed to be related to traversal of AudioProcessorGraph connections. Typically right before the exception was thrown, something like this:

Starting test: pluginval / Editor DPI Awareness...

*** FAILED: VALIDATION CRASHED

0: pluginval: juce::SystemStats::getStackBacktrace + 0x71
1: pluginval: `anonymous namespace'::getCrashLogContents + 0x51
2: pluginval: `anonymous namespace'::handleCrash + 0x13
3: pluginval: juce::handleCrash + 0x14
4: UnhandledExceptionFilter + 0x1ea
5: memset + 0x1b32
6: _C_specific_handler + 0x96
7: _chkstk + 0x11f
8: RtlRaiseException + 0x399
9: KiUserExceptionDispatcher + 0x2e
10: PluginvalTest: juce::HeapBlock<juce::AudioProcessorGraph::Node::Connection,0>::operator juce::AudioProcessorGraph::Node::Connection * + 0xa

Could It Be the AudioProcessorGraph?

My plugin that was crashing pluginval uses an AudioProcessorGraph internally as a member and has a small graph of 8 processors/nodes, 9 including the root AudioProcessorGraph.

So on a haunch I ended up altering the simple plugin test project to minimally replicate the internal use of an AudioProcessorGraph. And as soon as I did that the crash was back!

What I Discovered

If I used the following test setup I was able to reproduce the crash every time:

  • an AudioProcessor as a top-level plugin, fairly minimal
  • an AudioProcessorGraph that is a member of the top-level plugin, nothing special
  • two graph nodes:
    • a node for an AudioProcessorGraph::AudioGraphIOProcessor for audio output
    • a node for a TestProcessor that is also a dummby AudioProcessor for use as a test graph node
  • a single connection between the TestProcessor node and the AudioGraphIOProcessor node

If I instead only use the AudioProcessorGraph and added an AudioGraphIOProcessor node, no connections, no secondary TestProcessor node, I will still get the crash, but only sometimes (threading related?)

Steps to Reproduce

  1. Create a new plugin project in Projucer.
  2. Under the project settings I set plugin type to VST3, “Plugin is a Synth”, and use C++17.
  3. Add new files TestProcessor.cpp and TestProcessor.h
  4. Copy the source included in this forum post to the respective files: PluginProcessor.h/PluginProcessor.cpp, and TestProcessor.h/TestProcessor.cpp
  5. Build the plugin project.
  6. Run pluginval and add the test plugin
  7. Run tests at strictness 3 or higher

Note: I am building under Windows 10 64 bit, with Visual Studio 16.9.0; I built pluginval from source, and even upgraded its Juce modules to 6.0.7 to match the version I am using to build the plugins that are being tested just in case. I am more or less copying the techniques from: JUCE: Tutorial: Cascading plug-in effects.

Here is the source I am using:

PluginProcessor.h

#pragma once

#include <JuceHeader.h>
#include "TestProcessor.h"
using Node = juce::AudioProcessorGraph::Node;
using AudioGraphIOProcessor = juce::AudioProcessorGraph::AudioGraphIOProcessor;

//==============================================================================
/**
*/
class PluginvalTestAudioProcessor  : public juce::AudioProcessor
{
public:
    //==============================================================================
    PluginvalTestAudioProcessor();
    ~PluginvalTestAudioProcessor() override;

    //==============================================================================
    void prepareToPlay (double sampleRate, int samplesPerBlock) override;
    void releaseResources() override;

   #ifndef JucePlugin_PreferredChannelConfigurations
    bool isBusesLayoutSupported (const BusesLayout& layouts) const override;
   #endif

    void processBlock (juce::AudioBuffer<float>&, juce::MidiBuffer&) override;

    //==============================================================================
    juce::AudioProcessorEditor* createEditor() override;
    bool hasEditor() const override;

    //==============================================================================
    const juce::String getName() const override;

    bool acceptsMidi() const override;
    bool producesMidi() const override;
    bool isMidiEffect() const override;
    double getTailLengthSeconds() const override;

    //==============================================================================
    int getNumPrograms() override;
    int getCurrentProgram() override;
    void setCurrentProgram (int index) override;
    const juce::String getProgramName (int index) override;
    void changeProgramName (int index, const juce::String& newName) override;

    //==============================================================================
    void getStateInformation (juce::MemoryBlock& destData) override;
    void setStateInformation (const void* data, int sizeInBytes) override;

    //==============================================================================
    void setTestParam(float newValue);

private:
    std::unique_ptr<juce::AudioProcessorGraph> graphProcessor;
    Node::Ptr audioOutputNode;
    Node::Ptr testNode;

    void initGraphMinimalTest();
    void initGraphBasicTest();

    //==============================================================================
    JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (PluginvalTestAudioProcessor)
};

PluginProcessor.cpp

#include "PluginProcessor.h"

//==============================================================================
PluginvalTestAudioProcessor::PluginvalTestAudioProcessor()
#ifndef JucePlugin_PreferredChannelConfigurations
     : AudioProcessor (BusesProperties()
                     #if ! JucePlugin_IsMidiEffect
                      #if ! JucePlugin_IsSynth
                       .withInput  ("Input",  juce::AudioChannelSet::stereo(), true)
                      #endif
                       .withOutput ("Output", juce::AudioChannelSet::stereo(), true)
                     #endif
                       )
#endif
{
}

PluginvalTestAudioProcessor::~PluginvalTestAudioProcessor()
{
}

//==============================================================================
void PluginvalTestAudioProcessor::prepareToPlay(double sampleRate, int samplesPerBlock)
{
    auto numInputs = getMainBusNumInputChannels();
    auto numOutputs = getMainBusNumOutputChannels();
    if (graphProcessor == nullptr) graphProcessor = std::unique_ptr<juce::AudioProcessorGraph>(new juce::AudioProcessorGraph());
    graphProcessor->setPlayConfigDetails(numInputs, numOutputs, sampleRate, samplesPerBlock);
    graphProcessor->prepareToPlay(sampleRate, samplesPerBlock);

    // Pick ONE of the two test methods by uncommenting/commenting,
    // The 'basic' test causes the crash everytime, the 'minimal' test has an intermitent crash
    //-----------------------------------------------------------------------------------------
    initGraphBasicTest();       // crashes every time
    //initGraphMinimalTest();     // only crashes some times; requires multiple test runs to get crash
}

void PluginvalTestAudioProcessor::releaseResources()
{
    graphProcessor->releaseResources();
}

void PluginvalTestAudioProcessor::initGraphBasicTest()
{
    graphProcessor->clear();
    audioOutputNode = graphProcessor->addNode(std::make_unique<AudioGraphIOProcessor>(AudioGraphIOProcessor::audioOutputNode));
    testNode = graphProcessor->addNode(std::unique_ptr<AudioProcessor>(new TestProcessor()));

    for (int channel = 0; channel < 2; ++channel)
    {
        graphProcessor->addConnection({ { testNode->nodeID,  channel }, { audioOutputNode->nodeID, channel } });
    }
}

void PluginvalTestAudioProcessor::initGraphMinimalTest()
{
    audioOutputNode = graphProcessor->addNode(std::make_unique<AudioGraphIOProcessor>(AudioGraphIOProcessor::audioOutputNode));
}

//==============================================================================
const juce::String PluginvalTestAudioProcessor::getName() const { return JucePlugin_Name; }

bool PluginvalTestAudioProcessor::acceptsMidi() const
{
   #if JucePlugin_WantsMidiInput
    return true;
   #else
    return false;
   #endif
}

bool PluginvalTestAudioProcessor::producesMidi() const
{
   #if JucePlugin_ProducesMidiOutput
    return true;
   #else
    return false;
   #endif
}

bool PluginvalTestAudioProcessor::isMidiEffect() const
{
   #if JucePlugin_IsMidiEffect
    return true;
   #else
    return false;
   #endif
}

double PluginvalTestAudioProcessor::getTailLengthSeconds() const { return 0.0; }

int PluginvalTestAudioProcessor::getNumPrograms() { return 1; }

int PluginvalTestAudioProcessor::getCurrentProgram() { return 0; }

void PluginvalTestAudioProcessor::setCurrentProgram(int index) {}

const juce::String PluginvalTestAudioProcessor::getProgramName (int index) { return {}; }

void PluginvalTestAudioProcessor::changeProgramName(int index, const juce::String& newName) {}

#ifndef JucePlugin_PreferredChannelConfigurations
bool PluginvalTestAudioProcessor::isBusesLayoutSupported (const BusesLayout& layouts) const
{
  #if JucePlugin_IsMidiEffect
    juce::ignoreUnused (layouts);
    return true;
  #else
    if (layouts.getMainOutputChannelSet() != juce::AudioChannelSet::mono()
     && layouts.getMainOutputChannelSet() != juce::AudioChannelSet::stereo())
        return false;

   #if ! JucePlugin_IsSynth
    if (layouts.getMainOutputChannelSet() != layouts.getMainInputChannelSet())
        return false;
   #endif

    return true;
  #endif
}
#endif

void PluginvalTestAudioProcessor::processBlock (juce::AudioBuffer<float>& buffer, juce::MidiBuffer& midiMessages)
{
    juce::ScopedNoDenormals noDenormals;
    auto totalNumInputChannels  = getTotalNumInputChannels();
    auto totalNumOutputChannels = getTotalNumOutputChannels();

    for (auto i = totalNumInputChannels; i < totalNumOutputChannels; ++i)
        buffer.clear (i, 0, buffer.getNumSamples());
}

//==============================================================================
bool PluginvalTestAudioProcessor::hasEditor() const { return false; }

juce::AudioProcessorEditor* PluginvalTestAudioProcessor::createEditor() { return nullptr; }

//==============================================================================
void PluginvalTestAudioProcessor::getStateInformation (juce::MemoryBlock& destData) {}

void PluginvalTestAudioProcessor::setStateInformation(const void* data, int sizeInBytes) {}

//==============================================================================
juce::AudioProcessor* JUCE_CALLTYPE createPluginFilter() { return new PluginvalTestAudioProcessor(); }

TestProcessor.h

#pragma once
#include <JuceHeader.h>

class TestProcessor : public juce::AudioProcessor
{
public:
    //==============================================================================
    TestProcessor();
    ~TestProcessor() override;

    //==============================================================================
    void prepareToPlay(double sampleRate, int samplesPerBlock) override;
    void releaseResources() override;

    bool isBusesLayoutSupported(const BusesLayout& layouts) const override;

    void processBlock(juce::AudioBuffer<float>&, juce::MidiBuffer&) override;

    //==============================================================================
    juce::AudioProcessorEditor* createEditor() override;
    bool hasEditor() const override;

    //==============================================================================
    const juce::String getName() const override;

    bool acceptsMidi() const override;
    bool producesMidi() const override;
    bool isMidiEffect() const override;
    double getTailLengthSeconds() const override;

    //==============================================================================
    int getNumPrograms() override;
    int getCurrentProgram() override;
    void setCurrentProgram(int index) override;
    const juce::String getProgramName(int index) override;
    void changeProgramName(int index, const juce::String& newName) override;

    //==============================================================================
    void getStateInformation(juce::MemoryBlock& destData) override;
    void setStateInformation(const void* data, int sizeInBytes) override;

private:
    //==============================================================================
    JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR(TestProcessor)
};

TestProcessor.cpp

#include "TestProcessor.h"

//==============================================================================
TestProcessor::TestProcessor()
    : juce::AudioProcessor(BusesProperties()
        .withInput("Input", juce::AudioChannelSet::stereo(), true)
        .withOutput("Output", juce::AudioChannelSet::stereo(), true)
    )
{
}

TestProcessor::~TestProcessor() {}

//==============================================================================
const juce::String TestProcessor::getName() const { return "TestProcessor"; }

bool TestProcessor::acceptsMidi() const { return true; }

bool TestProcessor::producesMidi() const { return true; }

bool TestProcessor::isMidiEffect() const { return false; }

double TestProcessor::getTailLengthSeconds() const { return 0.0; }

int TestProcessor::getNumPrograms() { return 1; }

int TestProcessor::getCurrentProgram() { return 0; }

void TestProcessor::setCurrentProgram(int index) {}

const juce::String TestProcessor::getProgramName(int index) { return {}; }

void TestProcessor::changeProgramName(int index, const juce::String& newName) {}

//==============================================================================
void TestProcessor::prepareToPlay(double sampleRate, int samplesPerBlock) {}

void TestProcessor::releaseResources() {}

bool TestProcessor::isBusesLayoutSupported(const BusesLayout& layouts) const
{
    if (layouts.getMainOutputChannelSet() != juce::AudioChannelSet::mono()
        && layouts.getMainOutputChannelSet() != juce::AudioChannelSet::stereo())
        return false;

    return true;
}

void TestProcessor::processBlock(juce::AudioBuffer<float>& buffer, juce::MidiBuffer& midiMessages)
{
    juce::ScopedNoDenormals noDenormals;
    auto totalNumInputChannels = getTotalNumInputChannels();
    auto totalNumOutputChannels = getTotalNumOutputChannels();

    for (auto i = totalNumInputChannels; i < totalNumOutputChannels; ++i)
        buffer.clear(i, 0, buffer.getNumSamples());
}

//==============================================================================
bool TestProcessor::hasEditor() const { return false; }

juce::AudioProcessorEditor* TestProcessor::createEditor() { return nullptr; }

//==============================================================================
void TestProcessor::getStateInformation(juce::MemoryBlock& destData) {}

void TestProcessor::setStateInformation(const void* data, int sizeInBytes) {}
2

Unfortunately you’ll need to edit the graph a bit to get it working in production and get around little snags like this one.

I hit the same thing I cant remember exactly the culprit, but if you edit the prepare to play method on the graph class to create, initialize, and get the graph properly ready single threaded without the Async callbacks a lot of these issues go away.

3

Interesting, thanks for adding your experience. When you say:

… and get the graph properly ready single threaded without the Async callbacks a lot of these issues go away.

I wanted to clarify exactly what you meant. I’m not sure if you looked at my file sources in the initial post, but they’re about as simple as I could get them. There isn’t any introduced multi-threading, async callbacks, etc. except for what Juce and pluginval use internally.

This is the prepareToPlay() method:

void PluginvalTestAudioProcessor::prepareToPlay(double sampleRate, int samplesPerBlock)
{
    auto numInputs = getMainBusNumInputChannels();
    auto numOutputs = getMainBusNumOutputChannels();
    if (graphProcessor == nullptr) graphProcessor = std::unique_ptr<juce::AudioProcessorGraph>(new juce::AudioProcessorGraph());
    graphProcessor->setPlayConfigDetails(numInputs, numOutputs, sampleRate, samplesPerBlock);
    graphProcessor->prepareToPlay(sampleRate, samplesPerBlock);

    // Pick ONE of the two test methods by uncommenting/commenting,
    // The 'basic' test causes the crash everytime, the 'minimal' test has an intermitent crash
    //-----------------------------------------------------------------------------------------
    initGraphBasicTest();       // crashes every time
    //initGraphMinimalTest();     // only crashes some times; requires multiple test runs to get crash
}

And for example the minimal test (which still produces the crash, just not every time) is this:

void PluginvalTestAudioProcessor::initGraphMinimalTest()
{
    audioOutputNode = graphProcessor->addNode(std::make_unique<AudioGraphIOProcessor>(AudioGraphIOProcessor::audioOutputNode));
}

And the initGraphBasicTest() version isn’t much more complex, but it introduces a single L/R connection to a dummy processor that does nothing.

So I’m still not yet totally sure if the culprit for this crash is Juce, pluginval, some combination of their interaction around this scenario, or something else entirely.

For me, I suspect this is a possible bug somewhere in either pluginval or Juce framework and how they combine specifically around this scenario of having a nested/member AudioGraphProcessor that a member of the parent AudioProcessor implementing plugin. Although the stack trace is not super helpful, it does always seem to fire when Juce’s internals are iterating the graph’s nodes/connections when invoked by pluginval testing. It’s almost as if just Juce inherently iterating the graph is not thread-safe and yet pluginval is invoking tests in a threaded environment that will inherently invoke graph traversal and chance this crash.

4

Sorry I didn’t look super in depth at them just because the issue is likely not in your code and is in the JUCE code itself.

The graph is a very complicated class and it uses the message thread to create the rendering sequence, so the graph class itself is multi threaded.

The order you call things matters and many other little quirks of the class. I recommend stepping through the graph itself when you call addNode & prepareToPlay and you’ll see what I’m talking about. When you add nodes, it adds the node to a pool of nodes, and then triggers an async update which then creates the rendering sequence on the message thread. This can cause some strange behavior like the beginning of your audio stream not having effects applied to them.

1 Like
5

Wow, good to know. Thanks for the reply and I’m glad you’ve had some insight into this area before. I will check out the code and see what I make of it. It seems a little curious to me there doesn’t appear to be much of a mention of this in this tutorial: JUCE: Tutorial: Cascading plug-in effects.

If using the AudioProcessorGraph has so many potential pitfalls due to its nature, it would be great if JUCE outlined this somewhere; I didn’t see really anything that looked related to this in the docs for the class either.

It seems strange to me that my test code here is so simple, and based on the same principles in the tutorial, yet it will crash pluginval running standard tests.

Since my actual plugin (not this test code) is ultimately not that complicated, I guess maybe the best path forward is to just remove any use of AudioProcessorGraph and just “hardwire” all of the sequential buffer processing in the top-level plugin’s processBlock() implementation, I guess that’s what you probably meant in your original reply.

6

It’s been requested a lot :sweat_smile: they make some subtle improvements to it but there is likely a very small minority using the class and I’d guess that even roli / traktion are not using it at this point.

It is really a great piece of the framework so I wouldn’t overlook it there’s a ton of ways to use it that are undocumented like adding secondary inputs to nodes for modulation connections and easy midi routing across your app — but for actual use you’ll likely need to maintain a copy of it you edit yourself unfortunately.

Look at the forum more for the issues around latency and offline rendering — I wouldn’t say it’s a ton of work but you’ll need to put some days into understanding the class in it’s entirety to make full advantage of it.

For your pluginval thing — try initializing the graph with a default sample rate & blocksize in your constructor and then add your graph nodes then — then call samplerate and set play config etc again in prepare to play

7

I don’t know about Tracktion, but ROLI are definitely using the AudioProcessorGraph in their products. When I was working at ROLI, I spent quite a lot of time working on the APG and trying to ensure thread-safety in pluginval.

@Cymatic thanks for providing some example code. Having a code example makes it much faster for us to diagnose and resolve issues.

In this case, I ran the demo plugin under Pluginval at strictness 10, with thread sanitizer enabled. This allowed me to reproduce the data race issue you were seeing. It appears that prepareToPlay may be called from a background thread (other than the message thread), which then races with the graph’s asynchronous rebuilding.

In the demo project, I was able to completely resolve this issue by adding the statement juce::MessageManagerLock lock; to the top of initGraphBasicTest. This ensures that the graph’s rebuilding will never happen at the same time as calls which modify the graph from the background thread.

2 Likes
8

@reuk if the team is using it have you guys every noticed that offline renders don’t function correctly?

For example if you freeze & flatten the audio or do an offline export, the start of the audio stream is dry due to the async callback for graph construction after the prepareToPlay. There is also the overall graph latency issue, and some other quirks like no ability to disable graph reconstruction when you’re making a large amount of changes – i.e. adding a whole bunch of new connections on preset load.

I have a version which is slightly modified which addresses all of these, but I would strong recommend reviewing something like this in the processBlock as having offline exports not work correctly is a big issue:

void AudioProcessorGraph::processBlock (AudioBuffer<float>& buffer, MidiBuffer& midiMessages)
{
    if (((! isPrepared) && MessageManager::getInstance()->isThisTheMessageThread()) ||
        ((! isPrepared) && isNonRealtime())) {
        handleAsyncUpdate();
    }

    processBlockForBuffer<float> (buffer, midiMessages, *this, renderSequenceFloat, isPrepared);
}

The first line is from the original graph, but it’s basically to handle this offline issue – the issue is that the offline bounce is not always called from the message thread.

It’s a pretty minimal patch and imo is more clear than the original isThisTheMessageThread()

1 Like
9

I remember this being an issue for a while, but I thought it had been fixed. In processBlockForBuffer, when rendering offline, it looks like processing is postponed until the graph has been prepared.

To test, I modified the AudioPluginDemo so that the synth engine sits inside an AudioProcessorGraph instance:

struct Proc : juce::AudioProcessorGraph
{
    Proc()
    {
        setBusesLayout (BusesLayout { { juce::AudioChannelSet::mono() }, { juce::AudioChannelSet::mono() } });

        using IO = juce::AudioProcessorGraph::AudioGraphIOProcessor;
        auto audioOut = addNode (std::make_unique<IO> (IO::IODeviceType::audioOutputNode));
        auto midiIn = addNode (std::make_unique<IO> (IO::IODeviceType::midiInputNode));
        auto synth = addNode (std::make_unique<JuceDemoPluginAudioProcessor>());

        addConnection ({ NodeAndChannel { midiIn->nodeID, midiChannelIndex }, NodeAndChannel { synth->nodeID, midiChannelIndex } });
        addConnection ({ NodeAndChannel { synth->nodeID }, NodeAndChannel { audioOut->nodeID } });
    }

    bool isBusesLayoutSupported (const BusesLayout&) const override { return true; }
};

juce::AudioProcessor* JUCE_CALLTYPE createPluginFilter()
{
    return new Proc();
}

Then, I stuck a call to Thread::sleep (2000) at the top of AudioProcessorGraph::buildRenderingSequence() to simulate a time-consuming graph rebuild.

I built the plugin as a VST3 and rendered some audio through it in REAPER using “File → Render…”. The resulting audio file contained all the expected notes, including notes immediately at the beginning of the recording.

Did you encounter issues with a specific host or plugin format? I’ll take another look at this if you can provide a minimal example or a set of steps which trigger an issue. However, this appears to work correctly on JUCE develop.

10

Yeah let me make a test app for you, thank you so much for taking a look!

I imagine Reaper must be calling the offline render from the message thread every time then.

If you do have ableton handy – that was the DAW with the issue.

If you stick a isThisTheMessageThread() inside of the prepareToPlay – at least last I was debugging it – it was sometimes the message thread and sometimes not – why that is I have no idea.

So the repro for it was to freeze & unfreeze a few times. I was using the graph for an effects plugin and the first few blocks of audio would be dry as the graph constructed.

Edit:

I think your test app is perfect for this actually

11

Testing the app from above in Live 10 and 11, freezing seems to work correctly. Freezing then flattening shows a waveform which starts at the beginning of the track, in line with the first MIDI note. Freezing and unfreezing a few times also seems to work.

12

I must be losing my mind…

I’m not positive if this was stealthily fixed in develop or if I lost my mind chasing it down before, but I also just tried to repro it for about an hour and was unable to…

Apologies for wasting your time on that @reuk – I believe it was happening in the graph last year when we made a fork of it, but now I can’t get it to happen if I remove my fork and go back to the normal graph.

Thank you for sanity checking!

13

Alright, no worries. There have been a handful of tiny changes to the APG over the last year, so perhaps one of those fixed the issues you were seeing.

Glad it appears to be working now, and as always please let us know if you run into any issues in the future.

14

If you guys do end up being able to get something in the roadmap – other stuff I’ve added:

  • A function to query each nodes latency & return the total latency of the graph - this one is obviously tricky because if there are nodes in parallel this can be the wrong value.

  • A flag to disable topology change messages when making graph changes, I’m sure you noticed when you put that Thread::sleep(2000) in the build sequence things hung for much longer than 2000ms

Sorry for hijacking your thread @Cymatic !

3 Likes