iOS 11 FLAnimatedImage object leak. Bug

[papanton]

• ###

New Issue Checklist

• I have read and understood the CONTRIBUTING guide
• I have read the Documentation
• I have searched for a similar issue in the project and found none

Issue Info

Info	Value
Platform Name	iOS
Platform Version	11.0
SDWebImage Version	4.4.1
Integration Method	cocoapods
Xcode Version	Xcode 9
Repro rate	100%
Demo project link	https://github.com/papanton/SD_CrashProject

Memory grows uncontrollably during gif loading. Eventually, the app crashes due to memory issues. Memory warning never triggered. Demo project provided, hooked up to a source of gifs. Scroll on iOS 11 Device real device and it should crash. This issue occurs on iOS 11 Devices but not on iOS 10. On an old iPhone 5 running iOS 10 that I have, there are no issues.

After a bit of poking around with instruments, I realized there is a FLAnimatedImage leak. FLAnimatedImages are initialized but not deallocated.

Please advice, or let me know if you have any further questions.

This is instruments for iOS 10. See how the memory is getting deallocated:

This is ios 11 iPhone X. See how FLAnimatedImage has not been deallocating. App Crashed.

[dreampiggy]

The FLAnimatedImage instance, from 4.2, will bind to the memory cache UIImage using asssociate object, see UIImage+FLAnimatedImage. However, current we have no choice to disable this behavior. Will be added now and be available on v4.4.2

But, I'm confused that since we already add a UIApplicationDidReceiveMemoryWarningNotification for memory cache, which should clear all the memory cache immediatelly, until somebody hold a strong reference cause that associated FLAnimatedImage not dealloced. I'm now trying to investigate this behavior between iOS 10 & iOS 11.

I think something we done for FLAnimatedImage enhancemence is wrong. So I'll revert some change and provide a advanced control for each of thing we done, but not just hide all the detail behind.

[dreampiggy]

@papanton Hi. Have you tried to disable that weak memory cache introduced in #2379 ? You can test your demo with the latest commit of master branch using Cocoapods.

[papanton]

@dreampiggy I disabled the weak memory cache on the didFinish call in Appdelegate but this did not seem to fix the crash. One thing that might be creating problems is that the system memory warning never actually gets triggered. I updated my test project if you want to take a look.

[zhongwuzw]

@dreampiggy , these weak reference fix some problems.
@papanton , for the big GIF sets, I just suppose you set maxMemoryCost or maxMemoryCountLimit of SDImageCache to suitable value. Can you try it?

[dreampiggy]

@zhongwuzw I guess that setImageBlock in the previous version may retain the imageView, and will make the imageView alive longer than excepted. However, this setImageBlock retained self(imageView) will finally get released because this is not actually a retain cycle. When image load finished, the sd_setImageLoadOperation: associated operation will be removed. So that the imageView should be dealloced.

And also, if you don't disable FLAnimatedImageView.sd_cacheFLAnimatedImage option, this FLAnimatedImage instance will be stored to memory cache. Which is kept alive until you remove it or memory warning cause it been purged.

The insteresting behavior is that One thing that might be creating problems is that the system memory warning never actually gets triggered on iOS 11. Which may be the root case.

[papanton]

@zhongwuzw Setting the maxMemoryCountLimit actually works, but also limits the cache for regular images. I would prefer if I relied on the actual memory warnings for purge caching instead. If we find no solution I will probably go with your suggestion, too many crashes in production from this.

[zhongwuzw]

@dreampiggy Yeah, it's not retain cycle. Just would leads some extra objects alive and increase the memory.
TBO, I don't reproduce the crash, I use iPhone6+ and iOS11.3.1 and master branch. I Instruments it, and the VM allocation increase to 1.x G and then memory would down back to reasonable footprint. And also I received the memory warning.

[papanton]

@zhongwuzw are you using my test project? @dreampiggy have you been able to replicate from the issue? My memory increases to between 500m - 2.x G before the crash with an iPhone X. I just check forcing memory warnings and they work fine. Memory deallocates properly. I think the system for some reason doens't automatically trigger them,

[zhongwuzw]

@papanton Yeah, I use your prj to debug.

The insteresting behavior is that One thing that might be creating problems is that the system memory warning never actually gets triggered on iOS 11. Which may be the root case.

@dreampiggy , memory warning is not always triggered even the memory is going to be exhausted. Jetsam would kill any not good memory citizen.
Some situations would cause this, I know some of them, one is when the app allocate spikes memory, Jetsam would kill the app directly (I faced it before) . Another is runloop issue, because the notification needs to post by runloop, e.x. the code below show the simple infinite loop, it would leads to crash, it's extreme, but show some possible situations.

while(1)
{
    NSString *str = [NSString stringWithFormat:@"%@",self];
}

I don't know wether we can add the functionality to monitor memory consume. The benefit is we can manually remove the cached objects in low memory to prevent crash. The penalty is it would leads to occupy more CPU and battery energy. @dreampiggy What do you think? 🤔

[bpoplauschi]

@zhongwuzw I would not rely on NSString to always allocate memory, as the way strings are used, reused and allocated into global access memory is not totally known. I would feel better using another class, like NSNumber or NSData

[dreampiggy]

@papanton Sorry maybe I miss this issue. The demo you provide seems not works because of that API failed in this url : https://dashboard.stickerpop.co/api/animated. Could you provide a update and I'll check soon ?

{"message":"Unauthenticated."}

[papanton]

@dreampiggy opened up that endpoint, the project should work now. I apologize for that.

[dreampiggy]

@papanton I try the demo. With both iOS 10.3 && iOS 11.2 real device, it can trigger a OOM crash after many GIF images was shown. No difference between iOS 10 && iOS 11.

It seems that UIApplicationDidReceiveMemoryWarningNotification not been called when this use case OOM occured. This may because of that this notification is always be sent on the main queue by UIKit. Sometimes the OOM cause by Image/IO decoding process from global queue, so it's too late. Another issue is that if the current runloop is full of Core Animation task (GIF image in FLAnimatedImageView use a CALayer to show frames, which may consume and cause allocation of memory after Core Animation commit) , when the UIKit send the notification to the main queue, the system still run many allocation code for Core Animation and finally cause OOM to be killed. So there is no change to get that notification...

And another interestring issue, related to #2226 . In iOS 11.2~11.4, Apple introduce a bug that will cause crash because of decoding failed during Core Animation commit render. See #2226 and my bug report for Apple. During my test, I found that my iOS real device running iOS 11.2 will get this crash before the OOM crash or memory warning notification. So I guess maybe related to the difference between iOS 11 & iOS 10. Good news, this crash have already been fixed in iOS 12. (But Apple seems not fix this in iOS 11.4 :))

The better solution, maybe it's to limit the memory cache's size. You can try set SDImgeCache.maxMemoryCost or SDImageCache.maxMemoryCountLimit to a smaller size which is suitable for the device. (Calculate the number based on device total memory or dynamic change this propery)

In #2374, I'll try to provide more detail control about limit the memory usage. For example, you can disable store the FLAnimatedImage to memory cache (Using sd_cacheFLAnimatedImage property), cause it dealloc as soon as imageView dealloc.

[dreampiggy]

@papanton 4.4.2 is out. Could you please read me reply above (I analysis the reason and provide some help), and try the new version ?

To summary, you can disable the FLAnimatedImage memory cache by using FLAnimatedImageView.sd_cacheFLAnimatedImage = false, so that the image instance will dealloc immediately (But this may cause frame drop because of disk cache query on the main queue).

If you don't mind a short-term empty image during cell reusing, you can call sd_setImage(with: url, options: [.queryDataWhenInMemory]), this will query the disk cache asynchronously

If you still want to keep high frame rate without flashing, you'd better keep use cache, but you'd limit the cache size by maxMemoryCost and maxMemoryCount in SDImageCache. This will stop memory cache growth to high value.

[papanton]

Hey @dreampiggy thanks for all the help, I ended up setting the maxmemory cost and it seems to be performing well. You can close this issue.

[dreampiggy]

@papanton The animated image is a big task to talk about. If you're still care about the animated image solution , you can consider to try Animated Image in 5.0.0-beta. Which will not contains this issue because the we don't need to query disk cache during image display timing.