React-native-fast-image: Memory Leak on iOS: FastImage not released from memory when callbacks onLoad is set

Created on 14 Dec 2018  ·  17Comments  ·  Source: DylanVann/react-native-fast-image

An engineer at my company noticed a memory leak on iOS when the props onLoad is set on iOS.
FastImage doesn't get released when it should be.

After investigating the source code, I found out that using a strong reference to self to call sendOnLoad in sd_setImageWithURL.

I suspect this line to be the cause of this memory leaks.

https://github.com/DylanVann/react-native-fast-image/blob/141a4a02931ba9e02ee169ba4543e77d924b6a79/ios/FastImage/FFFastImageView.m#L167

I can submit a Pull Request to transform this strong reference into a weak reference, which should fix the memory leak.

All 17 comments

Sounds like a good idea to me 🤷‍♂️

Ok sorry for the delay, I did the fix on my fork not long after opening this issue, but I forgot to submit the PR. Now it's done I submitted the PR.

Hello @StevenMasini, I have tried your PR but it does not compile. (as I reported you in the PR)
What should I do?

Thank you,
Marco

@marf Hey sorry for the delay. I am working on it since yesterday. I found a very fast and easy way to reproduce the memory leak.

When you set the onError props in <FastImage>, but it's not really obvious yet what's causing this memory leak.

Also I am sorry for the fix I submitted earlier, it's been a while that I haven't done Objective C, so I forgot that typeof(self) will actually define the type as a pointer already.

So the correct syntax for the fix I submitted is __weak typeof(self) weakSelf = self; without the *.

But still, that's not enough to fix the memory leak. So I will keep investigating.

HI @StevenMasini , if the onError is not be set, there is no memory leak right?

@truongluong1314520 You can set the onError but just don't try to get any parameters from it.

onError don't pass any parameters.

For instance do

onError={() => {
// your implemention
}}

but don't

onError={error => {
// this will create a memory leak
// the same for every callback who doesn't pass parameters, just strictly follow the documentation
// don't try to retrieve parameters who doesn't exist
console.log(error)
}}

I would be happy if people could try my PR #433. It should fix the memory leak on iOS.

Let me know if it works for you or not ?

Hello @StevenMasini I will try your PR #433 , thank you!
In my case I was not using the onError callback so I do not think that my problem is related to it.
I mitigated the problem by downsizing the images from 1024x1024 to 512x512, in these case with lots of images it takes less memory, but it is a workaround.

I will try your PR #433 hoping memory usage decreases.

Thank you

@marf Actually not only the onError callback is affected, but all callbacks. So even using onLoad will lead to memory leaks.

It's good to reduce the resolution of your pictures, you probably didn't do it for nothing.
I hope that the fix I made will solve your problem, if not I am willing to help you out.

@StevenMasini thank you!

faced the same issue and had to remove the onError callback. Is there a fix coming for this?

@manithin What was your implementation of onError ? Did you try to retrieve a param from it ?

I remember that if you try to do this, it will lead to a memory leak.

onError={(error) => { ... }}

If so then just remove the param. onError does not return the specific error. If not, then that's another bug.

This is how i implement it. Just set an error status for displaying a default image.

onError={() => setImageError(true)}

when this block was disabled, there were immediate changes in the performance.

I would be happy if people could try my PR #433. It should fix the memory leak on iOS.

Let me know if it works for you or not ?

Heyy...@StevenMasini I am also facing same crash issue on ios. I want to user you PR. But don't know how can I use your PR. I don't know much more about git. I just installed fastimage from npm. Can you please tell me how I can use it ?

@kishanbharda
The memory leaks fixes have been merged into the current version of react-native-fast-image.

But if you are experiencing issues with image loading, etc. My branch still fix more issues.

You can modify your package.json to point at my branch with the following line

"@stevenmasini/react-native-fast-image": "^7.0.0"

Also, I keep that branch up to date with the main repository.

Thank you. I will try.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

mschipperheyn picture mschipperheyn  ·  3Comments

ryoid picture ryoid  ·  3Comments

Doko-Demo-Doa picture Doko-Demo-Doa  ·  3Comments

jslok picture jslok  ·  3Comments

Aligertor picture Aligertor  ·  3Comments