Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Image] EXC_BAD_ACCESS in [RCTImageDownloader _downloadDataForURL:progressBlock:block: #2437

Closed
walkaboutlanguages opened this issue Aug 25, 2015 · 9 comments
Labels
Resolution: Locked This issue was locked by the bot.

Comments

@walkaboutlanguages
Copy link

Scene with a horizontal ListView, containing photos occasionally produces this error.
#2 0x0000000109141259 in __62-[RCTImageDownloader _downloadDataForURL:progressBlock:block:]_block_invoke27 at /path/to/project/node_modules/react-native/Libraries/Image/RCTImageDownloader.m:107

Xcode screenshot attached for reference.

screen-shot-2015-08-25-at-2 23 32-pm

@brentvatne brentvatne changed the title EXC_BAD_ACCESS in [RCTImageDownloader _downloadDataForURL:progressBlock:block:] [Image] EXC_BAD_ACCESS in [RCTImageDownloader _downloadDataForURL:progressBlock:block: Aug 30, 2015
@fabianeichinger
Copy link
Contributor

I'm currently hitting this a lot with fast(-ish) changing GIFs loaded from asset bundles.
Especially iPhone 5s on iOS 9.1 (both simulated and real devices) seems prone to this.
I created a small project which reproduces this problem every time in a matter of seconds
https://github.com/feichngr/react-native-2437

@nicklockwood
Copy link
Contributor

That's very helpful, thanks! We've seen this reported a couple of times but I've had real trouble trying to reproduce it.

@nicklockwood
Copy link
Contributor

It looks like this is basically being caused by an OOM (Out Of Memory) happening inside the imageDecoder. The exact location of the error varies, but disabling the GIF decoder fixes it.

Our GIF decoder is very memory intensive because it decodes all of the image frames immediately instead of one-by-one as they play. This is better for performance, but worse for memory usage, and it can be disastrous for very large GIFs.

This example is super-helpful though, because it taxes the GIF decoder much more than any of our existing tests. I'll see what I can do to improve its memory performance.

@fabianeichinger
Copy link
Contributor

Thank you for looking into this so quickly.

I played around a bit more and I might be wrong (I'm not an iOS or ObjC programmer) but to me it looks
more like use-after-free or race condition or something similar, rather than OOM.

It sometimes fails even on an iPad Pro simulator - which should have plenty of RAM - after only cycling through 5-10 GIFs and Xcode only showing ~90 MB of memory usage.

I don't know if improving efficiency would alleviate this problem.

@nicklockwood
Copy link
Contributor

I understand your skepticism, but I spent several hours trying to diagnose the problem, and came to this conclusion after trying the following steps:

  1. I enabled NSZombies and ran the Leaks instrument to look for use-after free and memory leaks respectively. There were none.

  2. I repeatedly removed/modified the line of code that was crashing and found that each time I did so, the crash moved to somewhere else in the app.

  3. I modified the GIF decoder to only return the 1st image frame instead of the whole sequence, after which the app ran perfectly and I was unable to reproduce the crash.

@fabianeichinger
Copy link
Contributor

I'm sorry, but I'm very confident by now that this is caused by a race condition in which RCTImageLoader is deallocated before or while decodeImageData is called.
I updated my test project with a very simple case in which EXC_BAD_ACCESS is thrown most of the time (I was only able to try it on Xcode simulators, old emulated devices work best) and total size of all decoded frames should be less than 100 MB.

Maybe this is already fixed with the changes you made to RCTImageLoader recently?

@nicklockwood
Copy link
Contributor

RCTImageLoader is a bridge module whose lifetime is tied to the bridge - the only way it would be deallocated is if you Cmd-R to refresh, or kill the app.

It's possible that there is more than one bug at work here, but I ran your example against the latest codebase and still saw it crash in the same place shown in the original issue, as well as other places like the decoding step.

@nicklockwood
Copy link
Contributor

Well, you were right - it wasn't an OOM. I was accessing NSURLCache from two different threads. For some reason it was worse when decoding more image frames, but it would eventually crash even for individual images if you left it running long enough.

The good news is, it's fixed: 0fe074a

@fabianeichinger
Copy link
Contributor

Great to hear it's fixed. Thank you so much for taking the time. 👍

@facebook facebook locked as resolved and limited conversation to collaborators Jul 21, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Jul 21, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests

4 participants