RCTImageLoader crashes when canceling image loads (RN 0.36)

[SunnyGurnani]

Image Loader Crashing at OSAtomicOr32Barrier (EXC_BAD_ACCESS)

Start of Stack is

- (void)didMoveToWindow
{
  [super didMoveToWindow];

  if (!self.window) {
    // Cancel loading the image if we've moved offscreen. In addition to helping
    // prioritise image requests that are actually on-screen, this removes
    // requests that have gotten "stuck" from the queue, unblocking other images
    // from loading.
    **[self cancelImageLoad];**
  } else if ([self shouldChangeImageSource]) {
    [self reloadImage];
  }
}

Steps to Reproduce / Code Snippets

Put some Network Images on the Page (Specially in a List View) then scroll or change tabs so Images which are not loaded yet are out of the View Port and go back to same tab/viewport and you will see this error.

Expected Results

No Error

Additional Information

• React Native version: Latest 0.36.rc.0
• Platform(s) (iOS, Android, or both?): iOS
• Operating System (macOS, Linux, or Windows?): macOS

[hramos]

Thanks for submitting an issue. Can you update your original post and ensure all the fields required by the template are filled in?

[SunnyGurnani]

@hramos update the issue. Let me know if you need any more information.

[anttimo]

@SunnyGurnani you probably mean to have 0.36.rc.0 in the title instead of 0.30.rc.0?

[SunnyGurnani]

@anttimo thats correct I have changed the title

[SunnyGurnani]

This is also similar https://github.com/facebook/react-native/issues/10473

[chandlervdw]

This issue has us still using 0.32. I'm not sure why this hasn't become a more pressing issue to fix.

[pietropizzi]

Same as @chandlervdw here. We were constantly updating to the newest version to try if it is fixed. Now that we need to release an update to our app, we downgraded back to React Native 0.31 (for us 0.32 crashes as well with a related network issue) to be able to release an update. That was a sad moment since we also had to downgrade a couple of other libraries.

[mrspeaker]

Same boat - this is a huge issue for us, getting hundreds of crash reports each week from it. I was realllly hoping not to have to got back to 0.31 because there's so many other good bug fixes in there - so I come here daily hoping for a miracle patch ;)

[seanadkinson]

Similar story here. We upgraded to 0.33 awhile back, and found this issue during testing. I assumed it would be addressed within the month or two before we were ready to release, but now that date it quickly approaching, and I really don't want to downgrade.

Can this issue not be patched somehow, even something dirty? The entire app crashing is really not good... even if the images just didn't load, or didn't get cleaned up would be better.

I'll have to learn some more objective-c to see how I can help. Maybe there's someone who can do a quick technical writeup for why this is actually happening. For example, what is trying to be achieved with the line: OSAtomicOr32Barrier(1, &cancelled). Which part is the "BAD_ACCESS"? I assume the issue is the &cancelled pointer is no longer valid, but how or why does that happen? I'd love to help, but this language isn't my strong suit.

[hramos]

Is there a rnplay snippet that reproduces the crash?

[gitim]

@hramos I created an app to reproduce the bug https://github.com/gitim/ListViewApp. To reproduce you need scroll intensively, sometimes the bug happens quickly, sometimes you need to scroll more, I think it depends on your network connection.

[Aaang]

I can also confirm that this is still crashing, even though the crash rate got much better after upgrading to version 0.36.0.rc.1. We upgraded our production app from 0.28.0 to 0.34.0 two weeks ago. After that, we experienced an enormous increase of crashes, so we decided to release a new app update with 0.36.rc.1 as soon as possible (hence the release candidate and not the stable version) as there were two promising commits in 0.35.0. and 0.36.0,rc.1.

After evaluating the crash rate of this crash after a week on production, the crash rate was reduced by 88%, so it really helped a lot. But still, a proper fix would be amazing.

[gitim]

Did anybody try to disable removeClippedSubviews of ListView, does it reduce crashes?

[SunnyGurnani]

@gitim yes it does.

I also found that if the server has error or Not found for any image url in the list it increases these crashes even when removeClippedSubviews is disabled.

So I would suggest to set one of the URL which responds as 4xx or 5xx

[gitim]

I created a product pain. Please, vote to pay attention of the core team members to the bug.

[SunnyGurnani]

@gitim voted.

[javache]

Thanks @gitim, I'm trying to reproduce with your app right now. With Xcode's memory analyzer I can see a number of zombie NSCFURLSessionTask but I can't reliably repro the crash.

[gitim]

I didn't find reliable way to reproduce, but it is definitely reproducible in this app. After about 3 minutes of attempts it crashed http://www.giphy.com/gifs/l0MYw9QKCyg2ESvsI.

[javache]

@gitim: one theory I'm trying to verify now. Could you try removing the [_delegates removeObjectForKey:task]; in -[RCTHTTPRequestHandler cancel]? Since the task is still running at that point (and will still call the completion callback), this may lead to an early release.

The other thing I'm investigating is that when running your app in Instruments, it looks like all of the __NSCFLocalDataTask are actually being leaked.

[pietropizzi]

@gitim @javache I think what makes it easier to trigger a crash, is when you throttle the network speed on your machine ...

[arilitan]

I can confirm I am seeing this crash as well in my production app. on version 0.36. Was seeing it a lot more before I upgraded to 0.35. My app also has a Listview with many images that load and unload based on the user's scroll position.

[gitim]

@javache I have tried, it didn't help, the app is still crashing.

[javache]

Hey everyone, I've just put up https://github.com/javache/react-native/commit/fb2f5c50a912aa0dd4ea4551ac448d7b7e171940 which is a potential fix. I have not been able to consistently reproduce a crash, but I could find a significant amount of leaks in NSURLSessionTask which are usually indicating some state is messed up (and could cause crashes).

Summary of my changes:

• rewrote -[RCTImageLoader dequeueTasks] to be more thread-safe and efficient
• made RCTNetworkTask operations thread-safe
• made all RCTHTTPRequestHandler calls happen from the same dispatch queue and tweaked its cancellation logic

Please try out these changes by pointing your local react-native install at that github commit. Once we get some feedback that this solves the issues people have been seeing we'll land it on master and potentially do a new release of 0.37.

[pietropizzi]

Just tried it with our app and it is still crashing on this line. https://github.com/javache/react-native/blob/fb2f5c50a912aa0dd4ea4551ac448d7b7e171940/Libraries/Image/RCTImageLoader.m#L388

I would say that it took longer to crash than before.

[gitim]

I confirm, unfortunately, the bug is still here.

[runn3r85]

The bug is still there for us too ☹️

[unel]

@javache In situation when image was loaded and RCTImageView removed from superview in same time, main thread thinks that "cancelLoad" exists, but this variable already released in other thread =( //cc @Dmitry-Zherdev

[huitsing]

+1 on this issue, this is by far our biggest crash (on RN 0.36)

[runn3r85]

We've added scroll throttling to help lower crashes, but sadly hasn't fully stopped the issue.

scrollEventThrottle={200}

[animabear]

We replace the official ImageView(RCTImageView) to our own native imageview

const OurOwnImageView =  requireNativeComponent('OurOwnImageView', null);

[pietropizzi]

@javache Since this seems to be impossible to fix, what about a rollback of the changes that introduced this bug and go from there? It was introduced going from 0.31 to 0.32.

We are still stuck on 0.31 because of this bug and it is really limiting to us. Is there any way we can help with this?

[pietropizzi]

@animabear care to share your custom image View?

[animabear]

@pietropizzi our custom imageview wrote by oc, you can let your ios or android developer do it for you

https://facebook.github.io/react-native/docs/native-components-ios.html

[adeelraza]

+1 this is the biggest cause of crashes in our app.

[huitsing]

Hey everyone, we (ZeeMee) are willing to pay a $250 bounty to anyone who fixes this bug, and gets their fix merged into master! Let's kill this thing.

[gabegreenberg]

We (G2i) are going to match @huitsing 's $250 bounty!! So it's now at $500 total. That will buy you a very large turkey for your feast tomorrow (for those in the USA).

[huitsing]

Thanks @gabegreenberg ! Specifically, our crash looks like: Specifically, ours manifests as

Crashed: com.facebook.react.ImageLoaderURLRequestQueue
EXC_BAD_ACCESS KERN_INVALID_ADDRESS 0x0000000371c8beb8
---
RCTImageLoader.m line 258
__30-[RCTImageLoader dequeueTasks]_block_invoke

[agumack]

Hi,

I am using https://github.com/oblador/react-native-image-progress and I have never seen this error

[gabegreenberg]

@agumack what version of RN are you on?

[agumack]

@gabegreenberg from 0.2x and now 0.37

[SunnyGurnani]

I have use react-native-image-progress amd i have faced the crashes

Have you changed setting for removeClippedViews ?

On Wednesday, November 23, 2016, agumack notifications@github.com wrote:

@gabegreenberg https://github.com/gabegreenberg from 0.2x and now 0.37

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/facebook/react-native/issues/10433#issuecomment-262597016, or mute the thread https://github.com/notifications/unsubscribe-auth/AA5l2JxOuwKPLyCUDQNpGiod7swvMY_Oks5rBIfGgaJpZM4KZhDZ .

Thanks !

Sincerely, Sunny 917-804-7520

[pvsousalima]

I don't want to sound naive or not obvious but... Is this image loading cancelation really necessary? I mean... as you want them to load, not loading them would make a memory leak or a set of instructions for loading disposed? Me and my friend found a way to contour this and do not have a crash, and it worked for our purposes, however it does not look like a valid fix for the magnitude of this project

[brenordr]

@SunnyGurnani I was unable to reproduce the error in the RN 0.38 and 0.39-rc.0 Have someone tried these versions?

[mkonicek]

Knowing the commit(s) that introduced this crash might be a good start to debug and fix this.

@SunnyGurnani, @gitim Since you can reliably repro this, can you help us bisect? Can you try going to RN 0.31 and 0.32 to verify the bug was definitely introduced in 0.32? If we can be sure about the release where this was introduced, that only leaves us with ~250 commits, only a subset of which will be related to iOS and images.

@gabegreenberg You've emailed me saying your crash rate spiked recently. Was this because you upgraded to a new RN release and if yes, which one?

Here's the commit history for RCTImageLoader: https://github.com/facebook/react-native/commits/master/Libraries/Image/RCTImageLoader.m Version 0.32 was cut on Aug 8. Could any of the commits in July and beginning of August be related?

Upvoted the Product Pains post.

[mkonicek]

Anyone seeing production crashes - are they more likely to happen on a certain type of device? If yes that might help others reproduce this.

[brenordr]

@mkonicek Isn't related to a device, a friend was able to reproduce the issue even in the iOS simulator. ( @pvsousalima )

[janmonschke]

We just upgraded to RN0.38 and the issue seems to have gone away. We're not disabling removeClippedSubviews.

[Aaang]

We've experienced the first crashes related to [RCTImageLoader _loadImageOrDataWithURLRequest:size:scale:resizeMode:progressBlock:partialLoadBlock:completionBlock:]already with version 0.28.0, but really seldom.

Since we've directly upgraded to 0.33.0, the crash rate increased a lot, so I can at least confirm that the main crash cause was introduced with 0.33.0 or before.

[pvsousalima]

I also upgraded to RN0.38 and it doesn't crash for that reason anymore. But it broke our project, even after building it.

[mkonicek]

@rodriguesbreno - Great! Please bisect.

Knowing in which release exactly this started happening would help debug it. You can do that by downgrading to an earlier RN version - if you cannot repro, that version is good. If you can repro, that version is bad. Find the first bad one and comment here.

@javache Tells me he could repro the memory leak on a slow network, using the Network Link Conditioner to make network traffic slower.