Tutorial "Playing Sound Files" raises an exception on 2nd load

[Edy]

Hello,

I'm testing the Playing Sound Files tutorial on Visual Studio 2015. It's running on the debugger. When loading the first file it loads and play it fine. But right after opening a second file the program raises an "Access Violation" exception at ResamplingAudioSource::releaseResources().

The call stack points to the call transportSource.setSource in MainComponent::openButtonClicked:

    void openButtonClicked()
    {
        FileChooser chooser ("Select a Wave file to play...",
                             File::nonexistent,
                             "*.wav");                                        // [7]
        
        if (chooser.browseForFileToOpen())                                    // [8]
        {
            File file (chooser.getResult());                                  // [9]
            AudioFormatReader* reader = formatManager.createReaderFor (file); // [10]
            
            if (reader != nullptr)
            {
                readerSource = new AudioFormatReaderSource (reader, true);                // [11]
                transportSource.setSource (readerSource, 0, nullptr, reader->sampleRate); // [12]
                playButton.setEnabled (true);                                             // [13]
            }
        }
    }

The code above allocates readerSource with the new keyword. This code runs fine the first time, but the second time raises the described exception.

I've been able to fix the problem by releasing readerSource using the delete keyword before the new allocation:

void openButtonClicked()
    {
        FileChooser chooser ("Select a Wave file to play...",
                             File::nonexistent,
                             "*.wav");
        
        if (chooser.browseForFileToOpen())
        {
            File file (chooser.getResult());
            AudioFormatReader* reader = formatManager.createReaderFor (file);
            
            if (reader != nullptr)
            {
                if (readerSource != nullptr) delete readerSource;

                readerSource = new AudioFormatReaderSource (reader, true);
                transportSource.setSource (readerSource, 0, nullptr, reader->sampleRate);
                playButton.setEnabled (true);
            }
        }
    }

The above code works correctly without issues. 

I've noticed that readerSource is declared as:

ScopedPointer<AudioFormatReaderSource> readerSource

The JUCE Coding Standards discourage the use of the new and delete keywords. The ScopedPointer template automatically deletes the pointer when the object goes out of scope, but looks like it doesn't gets properly released in this case.

My questions are:

- Which is the correct way for reusing a pointer like in the above case? ScopedPointer + new raises an exception when the pointer is passed as argument to a method.

- Is my solution using new / delete valid? Is there a better solution?

Thank you very much!

Edy

[fr810]

Hi Edy,

you are absolutely right. The current version of the tutorial is broken but I don't think your fix is correct either as the ScopedPointer will already delete the readerSource when it is assigned a different pointer. Basically, the ScopedPointer (the current versoin of the tutorial) will do exactly what your code suggestion does internally. 

The reason why the code crashes is that the old readerSource object will be deleted (when the ScopedPointer is re-assigned, or - in your case - when you explicitely delete it) although the transportSource still has a reference to it. In fact, if you change the source with setSource, the transportSource will call releaseResources on the old readerSource object (which now has been deleted) - hence the crash.

You can call setSource (nullptr, ...) before re-assigning the ScopedPointer and this will fix the crash: by calling setSource (nullptr, ...) the transportSource will not have a reference to the readerSource anymore. Another way to fix this (without calling setSource twice) is the following:

if (reader != nullptr)
{
    ScopedPointer<AudioFormatReaderSource> newSource = new AudioFormatReaderSource (reader, true); // [11]
    transportSource.setSource (newSource, 0, nullptr, reader->sampleRate); // [12]
    playButton.setEnabled (true);                                             // [13]
    readerSource = newSource.release();
}

The above code will first assign the new readerSource to a temporary pointer (therefore not deleting the old one). We then call setSource on the transportSource. This will call releaseResources on the old readerSource and let go of any references to the old readerSource. We can then assign the new readerSource to the old readerSource's ScopedPointer. This will then delete the old readerSource.

Unfortunately, there is one additional complication: what happens if setSource or setEnabled throw an exception? Who will delete the new readerSource? To catch this edge case we also define newSource as a ScopedPointer on the stack. If setSource throws an exception then the newSource will be deleted as newSource exit's it's scope. However, if everything was succesful, we want to transfer the ownership of the newSource to the readerSource ScopedPointer. This is done with the release method (which let's go of the pointer without de-allocating it). 

We are about to update a whole bunch of tutorials and will include a revised version of the tutorial.

Thanks for spotting this!

Fabian 

 

[Edy]

Fabian, 

Thank you so much for your detailed response. I'm getting started with JUCE, so your explanation is very helpful to me in terms on coding standards and specific usage scenarios.

It's great to know that you're about to update the tutorials. I've found some links with the "coming soon" label, so I'll keep an eye on them as well :)

Thanks again!
Edy