Hitting assertion loading ZipFile from Binary Data


#1

I’m loading a file from a ZipFile which is stored in Binary Data.

I’m hitting the following assertion:

ZipFile::OpenStreamCounter::~OpenStreamCounter()
{
    /* If you hit this assertion, it means you've created a stream to read one of the items in the
       zipfile, but you've forgotten to delete that stream object before deleting the file..
       Streams can't be kept open after the file is deleted because they need to share the input
       stream that is managed by the ZipFile object.
    */
    jassert (numOpenStreams == 0);
}

When the following block finishes:

            {
                // Load from binary data

                MemoryBlock mb (BinaryData::freewordaudio_zip, BinaryData::freewordaudio_zipSize);

                std::unique_ptr<MemoryInputStream> inputStream = std::make_unique<MemoryInputStream> (mb, false);
                ZipFile audioZipFile (inputStream.release(), true);
                // do stuff with the file data
            }   // <----- SIGILL (signal SIGILL: illegal instruction operand)

What I don’t get, is that I’m releasing the InputStream pointer when passing it to the ZipFile constructor, and also telling it to delete the InputStream when it’s finished. So why is this assertion being hit?

I should also add that this is on Android, if it makes a difference…


#2

I think the inputStream you are releasing is not the one that triggers the assert. That one’s ownership is correctly transferred to the ZipFile instance. But one of the inputStreams you are reading from the zipFile (probably in the part // do stuff with the data) is still alive, when you release the ZipFile. In that moment all streams you created with ZipFile::createStreamForEntry() or similar become disfunctional, hence the assert.

Could that be the case?


#3

Yes, thanks for the clue. So the problem was elsewhere, in the // do stuff bit :slight_smile:

There I was pulling the data in with

audioZipFile.createStreamForEntry (*zipEntry)->readIntoMemoryBlock (wordAudioData);

Separating the calls fixed the issue:

        std::unique_ptr<InputStream> inputStream;
        inputStream.reset (audioZipFile.createStreamForEntry (*zipEntry));
        inputStream->readIntoMemoryBlock (wordAudioData);

It would be nice if createStreamForEntry already returned a std::unique_ptr to make this less verbose…


#4

Ok, so effectively you were leaking the returned stream :wink:

Indeed, I think there is universal agreement, that replacing raw pointers with smart pointers, that bear the information about ownership, is the route to go.
It will come, one fine day, I guess, but it’s up to the JUCE team to make that move…