Image: Keep track of contiguous buffer size to avoid heap buffer overflows
Is causing a memory leak on iOS when using filmstrips to render knobs/sliders.
For some reason this memory leak only actually exhibits when running on an actual device and the simulators don’t have the issue.
I’ve tried a bit to reproduce this, but without much luck so far. Some more details would be helpful to track down the issue.
How are you rendering the film strips? I’d assume you’re calling ImageCache::getFromMemory to load a filmstrip, Image::getClippedImage to extract the frame that you want to render, and then using Graphics::drawImage to render the clipped region. Is that accurate? Are you creating your own temporary graphics context anywhere?
What is your test procedure? I’ve tried putting together a test app that renders lots of clipped images, but the Instruments leak detector doesn’t find anything and Xcode’s built-in memory graph doesn’t seem to climb when left running for a while. I’ve tested on a physical iPad running iOS 15.4, and in the simulator.
Have you seen the same issue in any of the JUCE demos?
How did you find that the issue is in the linked commit? I ask because I don’t see any obvious changes to the memory management in this commit. Are you certain that the issue is present when building with this commit, but not present when building with the preceding commit?
Filmstrips are loaded via ImageCache::getFromFile() and the resulting image is cached until required.
I’m not calling getClippedImage(), I’m calling
void Graphics::drawImage (const Image& imageToDraw,
int dx, int dy, int dw, int dh,
int sx, int sy, int sw, int sh,
const bool fillAlphaChannelWithCurrentBrush) const
which internally clips the image.
My test procedure is to step over the above call whilst watching the memory usage. Before the particular checkin, and when running on mac or in simulator, the memory hardly changes. After the change I’m getting about 10M of memory allocated per call
No, but then I haven’t been building any to test this situation specifically
I pulled 6.1.5 and determined the issue wasn’t there. I then pulled 6.1.6 and started working backwards until the problem stopped occurring. I tested repeatedly to make sure this was correct and before this commit my app didn’t crash and stepping over the draw image call showed no extra memory usage, after it did.
ok, so looks like maybe the new size field is the problem - this isn’t a memory leak, just a massive allocation which is why I’m seeing it in one circumstance and not the other.
so instead of allocating 1888 * 561 bytes, it’s now allocating 135M ( 1888 * 71808 ) and the app is quickly running out of memory. 71808 is the height of the filmstrip.
So this functions behaviour has gone from allocating memory for the frame, to allocating the memory for the whole filmstrip. Is this the intended behaviour of this change or is this a bug?
So, looking at this I think the reason this is crashing on iOS on the ipad is because it’s running out of memory. The same problem would happen on other platforms if they didn’t have so much memory.
It seems strange that when we’re clipping a filmstrip and then working on it, the memory allocated for the image is still the filmstrip and not the clipped image - perhaps this needs some adjustment so the the “size” is correct for the clipped image?
Thanks, that’s helpful. The idea was to keep track of the available bitmap size, because in some cases “dataPtr + height * linestride” could end up pointing past the end of the allocated buffer. It sounds like allocating enough room for the entire rest of the image is unnecessary, though.
Please could you try changing
CFUniquePtr<CFDataRef> data (CFDataCreate (nullptr, (const UInt8*) srcData.data, (CFIndex) srcData.size));
to
const auto idealSize = (size_t) srcData.lineStride * (size_t) srcData.height;
const auto usableSize = jmin (idealSize, srcData.size);
CFUniquePtr<CFDataRef> data (CFDataCreate (nullptr, (const UInt8*) srcData.data, (CFIndex) usableSize));
and check whether it resolves the problem? Thanks!
