Crash on readIntoMemoryBlock

#1

I’ve got a crash I don’t understand. Can someone check I’m not doing anything stupid? It looks like, though doesn’t explicitly say in the documentation that (1) the MemoryBlock will be automatically resized and (2) allocation errors will throw or assert.

The crash is happening on readIntoMemoryBlock() where I get a write access error when the downloaded file exceeds a few Mb in size.

ServerOutcome LoopCloudApi::downloadCloudData(const String& downloadLink, const File& destination, URL::OpenStreamProgressCallback * callback, void * callbackContext)
{
	auto url = URL(downloadLink);
	int statusCode{ 0 };
	ScopedPointer<InputStream> in = url.createInputStream(false, callback, callbackContext, String(), 0, nullptr, &statusCode);

	if (in != nullptr && statusCode == 200)
	{
		MemoryBlock memory;

		auto bytesReceived = 0;

		for (;;)
		{
			auto b = in->readIntoMemoryBlock(memory, 63556);

			bytesReceived += int(b);

			if (callback)
			{
				auto shouldContinue = callback(callbackContext, bytesReceived, -1);

				if (!shouldContinue)
					return ServerOutcome::connectionError; // actually aborted by use
			}

			if (b == 0)
				break;
		}

The write error occurs here:

bool MemoryOutputStream::write (const void* const buffer, size_t howMany)
{
    jassert (buffer != nullptr);

    if (howMany == 0)
        return true;

    if (char* dest = prepareToWrite (howMany))
    {
        memcpy (dest, buffer, howMany); // << Write error!!
        return true;
    }

    return false;
}
#2

Very odd - looks fine to me, and we use MemoryInputStreams in hundreds of places to load much larger blocks than a few MB… Let us know if you find out any more clues.

#3

I thought it was a threading issue to start with - it’s on a second thread. But the thread isn’t being killed, and I’m assuming the functions I’m calling are okay to call outside the message thread.

So I’m just retrying a few times - see if it stops after exactly the same number of bytes each time…

#4

Nope - but some how the memory block in MemoryOutputStream is being set to NULL… which would be a problem.

Half way through the download this happens…

#5

Getting there … realloc returns a nullptr

#6

@jules … okay - so it cannot allocate the memory (for some reason yet to be determined - we are only using 800Mb overall for the applicaiton) so realloc returns nullptr. However it doesn’t throw - because the HeapBlock in MemoryBlock is set to not throw.

And then your check in MemoryOutputStream::write is duff:

if (char* dest = prepareToWrite (howMany))
{
    memcpy (dest, buffer, howMany);
    return true;
}

Because prepareToWrite adds the size onto a null ptr and therefore doesn’t return null as an error code.

Have a look and see if you agree!

#7

:sweat_smile: :sweat_smile:

#8

What’s so funny :wink: I’ve got a lot of data … :slight_smile:

#9

Ah, well yes… when you said “a few MB” I didn’t think you mean 800, that’s the kind of size where the OS could easily fail to allocate a contiguous block of that size.

#10

Sorry. I expressed that badly.

Our application is using 800Mb.

The file is 300Mb but it fails at about 30Mb in.

Anyway - I’ll improve my code so we don’t need so much contiguous memory. However the JUCE code could be better behaved when the realloc fails.

#11

Yes, good point. I guess it should really throw an exception there.

#12

I think it looked like you could return a nullptr somewhere and everything would be fine as well … anything better than what it does right now though - i’d settle for an exception but it’s not really the JUCE way :wink:

Anyway - I’d guess I’d better avoid doing what I’m doing for larger files…