Safely delete objects used in processBlock

I'm a little lost, I want to remove an object used in the audio callback from other trhead, but if I delete it and the object is currently processed by the audio callback, then I get an EXC_BAD_ACCESS (because the it points to nullptr)

Oscilloscope is a Component, it has Channels that read samples and then generates a Path. Oscilloscope access to Channels to get the paths to paint().

void Oscilloscope::Channel::preProcess (AudioBuffer <float> *buffer) noexcept
{
    if (! owner->isProcessing())
        return;

    if (buffer->getNumChannels() > 0)
    {
        buffer->clear();
        float* channelData = buffer->getWritePointer (0);

        for (int i = 0; i < buffer->getNumSamples(); ++i)
        {
            waveGenerator->process (&channelData[i]);
            channelData[i] *= owner->inputGain;
        }
    }
}

WaveGenerator is an abstract class for SineWaveGenerator, SquareWaveGenerator, etc. To change the waveGenerator type (waveGenerator is a ScopedPointer):

void Oscilloscope::Channel::setWaveGenerator (double frequency, int numWaves, WaveGeneratorType waveGeneratorType)
{
    waveGenerator = nullptr;
    if (waveGeneratorType == WaveGeneratorType::Sine
        waveGenerator = new SineWaveGenerator();
    else if (waveGeneratorType == WaveGeneratorType::Square)
        waveGenerator = new SquareWaveGenerator();
    //...etc...
    
    waveGenerator->prepareToPlay (owner->sampleRate);
    int roundedNumSamples = std::round (owner->sampleRate / frequency);
    double frequencyHz = owner->sampleRate / roundedNumSamples;

    fifoBuffer.resize (roundedNumSamples * numWaves);

    waveGenerator->set (frequencyHz);

 

preProcess is called by an AudioAppComponent class (Oscilloscope : Analizer : Component)

void MainContentComponent::getNextAudioBlock (const AudioSourceChannelInfo& bufferToFill)

{
    bufferToFill.clearActiveBufferRegion();
    AudioBuffer <float> buffer (2, bufferToFill.buffer->getNumSamples());
    buffer.clear();

    if (analizer != nullptr)
        analizer->preProcess (&buffer);

    for (int i=0; i < filterManagers.size(); i++)
        filterManagers[i]->processBlock (buffer);

    if (analizer != nullptr)
        analizer->process (&buffer);
}

 

Any help is appreciated

If I understand you correctly you want to switch the waveGenerator.

First idea instead of deleting and then (after some time) create a new waveGenerator, you could save the pointer, create a new waveGenerator and then delete the old waveGenerator:


void Oscilloscope::Channel::setWaveGenerator (double frequency, int numWaves, WaveGeneratorType waveGeneratorType) 
{ 
    WaveGenerator* oldGenerator = waveGenerator;
    /* waveGenerator = nullptr; */
    if (waveGeneratorType == WaveGeneratorType::Sine)
        waveGenerator = new SineWaveGenerator(); 
    else if (waveGeneratorType == WaveGeneratorType::Square) 
        waveGenerator = new SquareWaveGenerator(); 
    //...etc... waveGenerator->prepareToPlay (owner->sampleRate);
    delete oldGenerator; 

    int roundedNumSamples = std::round (owner->sampleRate / frequency); 
    double frequencyHz = owner->sampleRate / roundedNumSamples; 
    fifoBuffer.resize (roundedNumSamples * numWaves); 
    waveGenerator->set (frequencyHz);

But that does not help, if the object is deleted while the pointer in getNextAudioBlock is used...

You can solve this by using a ReferenceCountedObject: http://www.juce.com/doc/classReferenceCountedObject

When the object is referenced in getNextAudioBlock, the old waveGenerator stays alive until the reference goes out of scope and so the reference count drops to zero.

 

 

 

I haven't used JUCE's ReferenceCountedObject yet, but as a design approach it should work.

 

Thanks for your response, 

I used ReferenceCountedObject for other purposes in other project, I can not imagine a practical solution with it. Yes, I can check if counter is zero, then delete itself, but if in elsewhere I've created a new instance of WaveGenerator, it will not work, as it counts instances and references.

I imagine other ways, such as using flags: waveGenerator::remove() {removeFlag = true}, then in the processblock call to: waveGenerator::needsDeletion() {if (removeFlag==true) delete this;}, but I do not think it's a smart way to do it...

 

You don't delete the object, it will be deleted automatically, if there is no reference left.
I add some pseudo / untested code:

class WaveGenerator : public ReferenceCountedObject
{
    void processBlock();
    typedef ReferenceCountedObjectPtr<WaveGenerator> Ptr;
};

class MainContentComponent {
    // [...]

    void setWaveGenerator (double frequency, int numWaves, WaveGeneratorType waveGeneratorType) {
        if (waveGeneratorType == WaveGeneratorType::Sine) 
            waveGenerator = new SineWaveGenerator(); 
        else if (waveGeneratorType == WaveGeneratorType::Square) 
            waveGenerator = new SquareWaveGenerator();
        // no need to delete the old object, it is deleted automatically if no other reference is held

        // [...]
    }

    WaveGenerator::Ptr getWaveGeneratorPtr() {
        return waveGenerator;
    }

    void MainContentComponent::getNextAudioBlock (const AudioSourceChannelInfo& bufferToFill)
    {
        // this reference keeps our generator from beeing deleted, until wave goes out of scope
        WaveGenerator::Ptr wave = getWaveGeneratorPtr();

        // to be perfectly save, if you have no preselected object
        if (wave) {
            wave->processBlock();
        }
    }


private:
    WaveGenerator::Ptr waveGenerator;
};

The reference count in the getNextAudioBlock scope is exact the flag you thought of...

Hope this helps

 

I think I've confused about ReferenceCountedObject, I'll take a look again to this class.
I will try your example, Thanks!
 

Won't that go out of scope at the end of getNextAudioBlock and cause a deallocation on the audio thread? Perhaps consider the ReleasePool Timur proposes in his [url=https://github.com/CppCon/CppCon2015/raw/master/Presentations/C%2B%2B%20In%20the%20Audio%20Industry/C%2B%2B%20In%20the%20Audio%20Industry%20-%20Timur%20Doumler%20-%20CppCon%202015.pdf]CppCon presentation[/url] (pp 67-70).

 

Yes, the reference goes out of scope, and if there was no change in the object which holds the second reference, nothing happens.

But you are right, if deconstruction is problematic in terms of timing (release is much less problematic than allocating, right?), then you need something else, like a third reference (probably in the ReleasePool, I forgot the details of Timur's proposal) and asynchronous checks, if this is the only one holding a reference and dropping it in that case.

If WaveGenerator is not a huge objects with much of data in it, the cost of building and maintaining the release pool might eventually be saved.

But the ReleasePool is a good idea for that, didn't think of it...

Yes,  +1 to Andrew's comment, the asynchronous ReleasePool is the solution!

Hopefully I'll have the time to add a JUCE-ified version of that ReleasePool to JUCE at some point.

Hey I have a similiar / the same problem and found this thread here.

I couldn’t find any material or example of such a “Release Pool” in action. There is this
ScopedAutoReleasePool class in Juce, but the documentation didn’t really get me far.

So any tipps / code snippets on how to make this work properly? :slight_smile:

Thanks in advance!

You could also use a shared_ptr with a custom deleter. In the custom deleter you can then choose to delete the object on a different thread.

/* static method */
void MyClass::deleteOnMessageThread( AudioObject* ptr )
{
	//The super intelligent awesome smart pointers even delete nullptrs. HOORAY!! \o/
	if( ptr != nullptr )
	{
		juce::MessageManager::callAsync([ptr](){
			delete ptr;
		});
	}
}

std::shared_ptr<AudioObject> object;
object.reset(new AudioObject(), MyClass::deleteOnMessageThread);
1 Like

This is the link again to the talk. I think all code for the release pool is in there.

Hey Rebbur thank you very much for the fast reply, the shared pointer looks good, too!

But What I don’t fully understand is, if I start to delete my object from the message thread couldn’t that mess with my audio thread process?

So I have an OwnedArray of Objects and once an object is not longer needed (faded out fully) I tell the message thread to delete this object when possible (since callinc OwnedArray.remove() from the audio thread directly is illegal as far as I understand?)

But if my audio thread then just started its next iteration over my objects and the message thread deletes the no longer needed one then, doesn’t that mess with my loop by changing the size of the array and thus the pointers of the OwnedArray? So Element 3 becomes 2 mid loop and might be skipped over or something?

Or this handled behind the curtains by the smart pointer / release pool so this can’t happen?

Thanks again!

Just thinking about it myself again:

Maybe an OwnedArray isn’t the way to go here, since we have 2 Threads “handling” the Data, although one is only there for deleting. So maybe a ReferenceCountedArray could help me out here?

But my question about the changing of the size of an array mid-loop still is open :slight_smile:

The way the std does it using std::remove_if:

std::remove_if moves the elements you want to delete to the end. When that is done, the iterator to the first element to be deleted is fed to the std::erase, where the actual removal and destruction happens by simply resizing the vector.

Back to the auto release pool:

You use a vector containing ReferenceCountedOpbect::Ptr. That way the objects cannot be deleted anywhere else.
A Timer continuously tests for elements, that are nowhere else referenced but here (ref-count == 1). Those are to be deleted.

Here is a possible implementation:

#include <JuceHeader.h>

class AutoReleasePool : private juce::Timer
{
public:
    using Ptr = juce::ReferenceCountedObjectPtr<juce::ReferenceCountedObject>;
    AutoReleasePool()
    {
        startTimerHz (1);
    }

    void addToPool (Ptr object)
    {
        for (auto& item : pool)
            if (item.get() == object.get())
                return;

        pool.push_back (object);
    }

    int getNumObjects() const
    {
        juce::MessageManagerLock lock;

        return int (pool.size());
    }

private:
    void timerCallback() override
    {
        pool.erase (std::remove_if (pool.begin(), pool.end(),
                                    [](auto& item)
                                    {
                                        return item->getReferenceCount() < 2;
                                    }),
                    pool.end());
    }

    std::vector<Ptr> pool;
    JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (AutoReleasePool)
};

class FooObject : public juce::ReferenceCountedObject
{
public:
    FooObject() = default;

    using Ptr = juce::ReferenceCountedObjectPtr<FooObject>;

private:
    JUCE_DECLARE_NON_COPYABLE_WITH_LEAK_DETECTOR (FooObject)
};

//==============================================================================
int main (int argc, char* argv[])
{
    juce::ScopedJuceInitialiser_GUI app;

    AutoReleasePool pool;

    {
        FooObject::Ptr object (new FooObject());
        pool.addToPool (object);

        for (int i = 0; i < 5; ++i)
        {
            DBG ("Elements in pool " << pool.getNumObjects());
            juce::Thread::sleep (500);
            juce::MessageManager::getInstance()->runDispatchLoopUntil (500);
        }
    }

    juce::MessageManager::getInstance()->runDispatchLoopUntil (1000);

    DBG ("Elements in pool " << pool.getNumObjects());

    return 0;
}
1 Like

thank you once again daniel for the insight!

So the ReleasePool is the best solution for my purpose I guess?

The ReleasePool is a standard solution to make sure, the deletion of objects happens only in one place/from one thread. You could even move the deletion to a background thread, it doesn’t have to be the messageThread (simply use a HighResolutionTimer instead of Timer, which will use a dedicated thread). But the time needed for destroying objects should not matter in the low priority thread.

To use it correctly, you copy lists of Object::Ptr to be processed around. Having the list (NOT a reference to the list) in the processBlock() scope ensures the objects are alive during the execution. And the reference in the AutoReleasePool makes sure when the list in processBlock() goes out of scope, that no destruction happens.

1 Like

I see.
Is a ReferenceCountedArray a suited candidate for such a “list” or should I rely on something like std::vector < ReferenceCountedObject::Ptr > ?
(Sorry if this is a stupid question, just starting to dive into “thread save jucing” :wink: )

They are equally well suited.
Since I started in 2015 with JUCE more and more containers were replaced with the std ones, that’s why I prefer now the std, although the API of some juce containers have advantages. It is totally a matter of personal preference.

Honestly, in my video engine I even use std::vector<std::shared_ptr<foleys::AVClip>> clips;

Ok :slight_smile:

So in a first fast attempt to implement your class, I still get the BAD_EXCESS problem (but atleast it is compiling and still working!)

“To use it correctly, you copy lists of Object::Ptr to be processed around”

Do I have to create a local copy of my ReferenceCountedArray to achieve this?

So in getNextAudioBlock() do I have to do something like the following or am I understanding this wrong?

ReferenceCountedArray < myObject > localCopyOfList = theRealFilledList;
for( auto* element : localCopyOfList)
{
...
}

Basically what I’m trying to achieve is having a “matrix” object, that can copy samples from an input with a smoothed value to an output. So when a new connection is made, I create a new connection object instance which processes its samples (that all works fine) and when it’s no longer needed, for example after loading a different set of connections and fading out, I want to delete it (facing the problems I’m describing).
Maybe there is even a completly different design solution for this, without dynamically allocating memory? :roll_eyes:

That is actually how I would have done it.
Only caveat: at the moment of copying theRealFilledList, the UI thread could alter it, so there should be a critical section around. I know, that is a lock, but I think if it is not shared with anything else, it should be fine. There are different opinions of course, and it’s always a trade off.

1 Like