Bad access on shutdown/destructor for ScopedPointer AudioFormatReader

General JUCE discussion
1

This may well be a lack of fully understanding RAII techniques, but I’m hitting a bad access error on shutdown in a small audio playback app in juce_ContainerDeletePolicy.h:

template <typename ObjectType>
struct ContainerDeletePolicy
{
    static void destroy (ObjectType* object)
    {
        // If the line below triggers a compiler error, it means that you are using
        // an incomplete type for ObjectType (for example, a type that is declared
        // but not defined). This is a problem because then the following delete is
        // undefined behaviour. The purpose of the sizeof is to capture this situation.
        // If this was caused by a ScopedPointer to a forward-declared type, move the
        // implementation of all methods trying to use the ScopedPointer (e.g. the destructor
        // of the class owning it) into cpp files where they can see to the definition
        // of ObjectType. This should fix the error.
        ignoreUnused (sizeof (ObjectType));

        delete object; // <---- EXC_BAD_ACCESS here
    }
};

If I use only use a class member ScopedPointer<AudioFormatReader> formatReader,
or - if I use a function variable of the same type e.g.

ScopedPointer<AudioFormatReader> reader = formatManager.findFormatForFileExtension (format)->createReaderFor (soundBuffer, true);

and then assign it to the member with formatReader = reader.release() I get a bad access error on the AudioFormatReader destructor trying to delete input.

The full code of the function:

    void loadNewSample (const void* data, int dataSize, const char* format)
    {
        MemoryInputStream* soundBuffer = new MemoryInputStream (data, static_cast<std::size_t> (dataSize), false);
        
        ScopedPointer<AudioFormatReader> reader = formatManager.findFormatForFileExtension (format)->createReaderFor (soundBuffer, true);

        ScopedPointer<AudioFormatReaderSource> newSource = new AudioFormatReaderSource (reader, true);

        transportSource.setSource (newSource, 0, nullptr, reader->sampleRate);
        playButton.setEnabled (true);
        thumbnail.setReader(reader, generateHashForSample(data, 128));
        readerSource = newSource.release();  
        formatReader = reader.release();
    }
2

I think reader is already held and deleted by the thumbnail when you pass it with setReader(). So you’re probably deleting it twice

3

You can use raw pointers for those things the new source takes ownership. Or call release inside:

ScopedPointer<AudioFormatReaderSource> newSource = new AudioFormatReaderSource (reader.release(), true);

Otherwise there is a time when two objects claim ownership. And newSource is allowed to delete the reader before you call the release.

The call release at the end is not how it is done. In a perfect world you would never need to call release. If you wanted to delete the objects at the end, simply set the ScopedPointer to nullptr:

readerSource = nullptr;  // deletes the readerSource
formatReader = nullptr;  // deletes the formatReader

while

readerSopurce.release();  // leaves a dangling pointer

And you even don’t need to set it to nullptr, because when the ScopedPointer on the stack goes out of scope, it deletes the object it points to.

4

Thanks, some good clues, but I’m still getting the exception.
My current code for the function:

    void loadNewSample (const void* data, int dataSize, const char* format)
    {
        MemoryInputStream* soundBuffer = new MemoryInputStream (data, static_cast<std::size_t> (dataSize), false);
        
        AudioFormatReader* reader = formatManager.findFormatForFileExtension (format)->createReaderFor (soundBuffer, true);
        AudioFormatReaderSource* newSource = new AudioFormatReaderSource (reader, true);

        transportSource.setSource (newSource, 0, nullptr, reader->sampleRate);
        playButton.setEnabled (true);
        thumbnail.setReader(reader, generateHashForSample(data, 128));
        readerSource = newSource; // <-- ScopedPointer<AudioFormatReaderSource> readerSource;
    }
5

should be new AudioFormatReaderSource (reader, false); I think

I haven’t tested it, but I would probably write like :

void loadNewSample (const void* data, int dataSize, const char* format)
{
    MemoryInputStream* soundBuffer = new MemoryInputStream (data, static_cast<std::size_t> (dataSize), false);

    ScopedPointer<AudioFormatReader> reader = formatManager.findFormatForFileExtension (format)->createReaderFor (soundBuffer, true);

    if (reader != nullptr)
    {
        // readerSource is a ScopedPointer
        if (readerSource = new AudioFormatReaderSource (reader, false))
        {
            transportSource.setSource (readerSource, 0, nullptr, reader->sampleRate);
            playButton.setEnabled (true);
            thumbnail.setReader (reader.release(), generateHashForSample (data, 128));
        }
    }
}
2 Likes
6

Thanks, changing that parameter to false sorted out the crash, and your code suggestion looks good. Cheers!