Raster/TexturePoolHolder leaks textures

[jonkan]

We're seeing a lot of "out of memory"-crashes after a while of heavy panning (iPad Air 2, a raster only stylesheet containing two raster-layers, both using 1024px tiles).

Trying to track down what's eating all the memory I found that in gl_object_store.cpp, TexturePoolHolder::reset() it seems textures are never deleted due to the line if (!bool()) return;.
There's also the line if (bool()) return; which to me seems to have no purpose?
Or am I missing something?

I can also add that, with those lines removed TexturePoolHolder::reset() seems to always be called having a NULL objectStore when called from Raster::~Raster()...

[jfirebaugh]

if (bool()) originates in 1241730 -- cc @brunoabinader.

[jonkan]

Tested with latest master now and the crashes are easily reproducible with the "iosapp"-target and the default "Satellite"-style by constantly panning in one direction, takes a while though..

[brunoabinader]

There's also the line if (bool()) return; which to me seems to have no purpose? Or am I missing something?

Oh my, this looks like my eyes missed this while batch replacing strings on my IDE. Will send a patch soon.

[brunoabinader]

Checking the code, the bool() is not a typo - it is using the bool() from https://github.com/mapbox/mapbox-gl-native/blob/master/src/mbgl/gl/gl_object_store.hpp#L114.

[jonkan]

Oh, that I'd missed.. I tested again with a clean master (as I had commented out just that line last time) and the memory-leak/crashing is still there.

[brunoabinader]

@jonkan can you please verify the patches from #5141?

[jonkan]

@jonkan can you please verify the patches from #5141?

👍 For the purpose of readability if (!created()) return; is more clear to me than if (!bool()) return;, not sure I understand if the PR was intended to fix more than just the readability aspect. The memory-leaking/crashing is still present, but might have its cause somewhere else..

[brunoabinader]

Reopening for verification - @jonkan can you please verify?

[friedbunny]

We will want to include an iOS changelog entry for this fix, as it affected iOS 3.2.x.

Android 4.0.x is also affected, but I’m not sure if 4.1.0 will include this? /cc @bleege

[bleege]

@friedbunny Thanks for the ping. I branched 4.1.0-beta.1 off of 26a425f before the final two commits ( b2b2797 and c111250 ) had been pushed to master. We'll have to cherry pick them to the release-android-v4.1.0 branch when it's been verified and @brunoabinader closes this issue.

/cc @tobrun @cammace

[jonkan]

@brunoabinader Awesome! That solved it, not seeing any crashes or leaks. 👍

[brunoabinader]

Thank you @jonkan!

[bleege]

Thanks for confirming and getting this fixed @jonkan @brunoabinader. I'll cherry pick the remaining two commits into the release-android-v4.1.0 branch now so that they'll ship with future 4.1.0 builds and releases.

[brunoabinader]

Awesome @bleege, thanks!

[bleege]

Cherries have been picked!

[friedbunny]

Updated the iOS changelog in 2f25c1a — this issue will be fixed in the v3.3.0 release (and whichever pre-release comes after alpha 3).