Skip to content

Cancelling offscreen image loads causing crashes in 0.33.0-rc.0 #9751

New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom
Closed
lprhodes opened this issue Sep 6, 2016 · 2 comments
Closed

Cancelling offscreen image loads causing crashes in 0.33.0-rc.0 #9751

lprhodes opened this issue Sep 6, 2016 · 2 comments
Labels
Resolution: Locked This issue was locked by the bot.

Comments

@lprhodes
Copy link
Contributor

lprhodes commented Sep 6, 2016

Issue Description

I'm using RN 0.33.0-rc.0 and have a standard list view that continues to load more rows (using Relay) as the user scrolls down:
simulator screen shot 6 sep 2016 10 41 59

If you scroll through the list on device (iPhone 6S) I'm frequently seeing crashes - all look to be related to the cancelling of loading images once the view has gone off screen.

Crash Traces
screen shot 2016-09-06 at 10 45 15
screen shot 2016-09-06 at 10 45 02
screen shot 2016-09-06 at 10 44 46
screen shot 2016-09-06 at 10 44 28
screen shot 2016-09-06 at 10 44 16

Expected Results

Deallocated instances not called or instances to be retained

Additional Information

  • React Native version: 0.33.0-rc.0
  • Platform(s) (iOS, Android, or both?): iOS (not tested Android)
  • Operating System (macOS, Linux, or Windows?): iOS 10
@lprhodes lprhodes changed the title Cancelling of offscreen image loads causing crashes in 0.33.0-rc.0 Cancelling offscreen image loads causing crashes in 0.33.0-rc.0 Sep 6, 2016
@ymmuse
Copy link
Contributor

ymmuse commented Sep 8, 2016 •
edited
Loading

I have met same crashes in RN 0.32.0 #9334

@tobycox tobycox mentioned this issue Sep 21, 2016
Closed
@ghost ghost closed this as completed in 1a62b66 Sep 21, 2016
@pfeiffer pfeiffer mentioned this issue Sep 28, 2016
Closed
facebook-github-bot pushed a commit that referenced this issue Sep 28, 2016
@pfeiffer
Use strong reference in RCTImageLoader
b2dac83 
Summary:
This PR is related to the multitude of crashes (#10016, #9751, #9882).

From my understanding, we should be using a strong reference when calling `decodeImageData` or we could be calling the method on a deallocated instance.

PR #9751 have mitigated this by adding a fail-safe, but I think the culprint is the weak reference, which this PR fixes.

Tested on iOS only, since it doesn't touch Android.
Closes #10147

Differential Revision: D3938763

fbshipit-source-id: 7389d4ae7a98926014401a1fe0cbbdcdd5ee6a01
cmcewen pushed a commit to cmcewen/react-native that referenced this issue Sep 28, 2016
@pfeiffer @cmcewen
Use strong reference in RCTImageLoader
936bbb9 
Summary:
This PR is related to the multitude of crashes (facebook#10016, facebook#9751, facebook#9882).

From my understanding, we should be using a strong reference when calling `decodeImageData` or we could be calling the method on a deallocated instance.

PR facebook#9751 have mitigated this by adding a fail-safe, but I think the culprint is the weak reference, which this PR fixes.

Tested on iOS only, since it doesn't touch Android.
Closes facebook#10147

Differential Revision: D3938763

fbshipit-source-id: 7389d4ae7a98926014401a1fe0cbbdcdd5ee6a01
cmcewen pushed a commit to cmcewen/react-native that referenced this issue Sep 28, 2016
@tobycox @cmcewen
FIX facebook#9751 - Cancelling offscreen image loads causing crashes
0497817 
Summary:
This fixes facebook#9751

We were seeing the same issues as the original reporter.

I'm not an expert on this code, so apologies if it's naive, but we haven't seen the crash since making this change.

From my understanding of it, it seems like the `cancelLoad` block was being released before the block returned by `_loadImageOrDataWithURLRequest:` was called.
This PR checks the `cancelled` flag before calling `cancelLoad()`.
Closes facebook#10016

Differential Revision: D3902723

Pulled By: javache

fbshipit-source-id: 75cd115e28694105c6fc29469986998ca0d4cd09
mikelambert pushed a commit to mikelambert/react-native that referenced this issue Sep 29, 2016
@tobycox @mikelambert
FIX facebook#9751 - Cancelling offscreen image loads causing crashes
7cc9224 
Summary:
This fixes facebook#9751

We were seeing the same issues as the original reporter.

I'm not an expert on this code, so apologies if it's naive, but we haven't seen the crash since making this change.

From my understanding of it, it seems like the `cancelLoad` block was being released before the block returned by `_loadImageOrDataWithURLRequest:` was called.
This PR checks the `cancelled` flag before calling `cancelLoad()`.
Closes facebook#10016

Differential Revision: D3902723

Pulled By: javache

fbshipit-source-id: 75cd115e28694105c6fc29469986998ca0d4cd09
mikelambert pushed a commit to mikelambert/react-native that referenced this issue Sep 29, 2016
@pfeiffer @mikelambert
Use strong reference in RCTImageLoader
25ce67e 
Summary:
This PR is related to the multitude of crashes (facebook#10016, facebook#9751, facebook#9882).

From my understanding, we should be using a strong reference when calling `decodeImageData` or we could be calling the method on a deallocated instance.

PR facebook#9751 have mitigated this by adding a fail-safe, but I think the culprint is the weak reference, which this PR fixes.

Tested on iOS only, since it doesn't touch Android.
Closes facebook#10147

Differential Revision: D3938763

fbshipit-source-id: 7389d4ae7a98926014401a1fe0cbbdcdd5ee6a01
gre pushed a commit to gre/react-native that referenced this issue Oct 10, 2016
@tobycox @gre
FIX facebook#9751 - Cancelling offscreen image loads causing crashes
8281298 
Summary:
This fixes facebook#9751

We were seeing the same issues as the original reporter.

I'm not an expert on this code, so apologies if it's naive, but we haven't seen the crash since making this change.

From my understanding of it, it seems like the `cancelLoad` block was being released before the block returned by `_loadImageOrDataWithURLRequest:` was called.
This PR checks the `cancelled` flag before calling `cancelLoad()`.
Closes facebook#10016

Differential Revision: D3902723

Pulled By: javache

fbshipit-source-id: 75cd115e28694105c6fc29469986998ca0d4cd09
gre pushed a commit to gre/react-native that referenced this issue Oct 10, 2016
@pfeiffer @gre
Use strong reference in RCTImageLoader
9b19041 
Summary:
This PR is related to the multitude of crashes (facebook#10016, facebook#9751, facebook#9882).

From my understanding, we should be using a strong reference when calling `decodeImageData` or we could be calling the method on a deallocated instance.

PR facebook#9751 have mitigated this by adding a fail-safe, but I think the culprint is the weak reference, which this PR fixes.

Tested on iOS only, since it doesn't touch Android.
Closes facebook#10147

Differential Revision: D3938763

fbshipit-source-id: 7389d4ae7a98926014401a1fe0cbbdcdd5ee6a01
grabbou pushed a commit that referenced this issue Oct 10, 2016
@pfeiffer @grabbou
Use strong reference in RCTImageLoader
cf906cc 
Summary:
This PR is related to the multitude of crashes (#10016, #9751, #9882).

From my understanding, we should be using a strong reference when calling `decodeImageData` or we could be calling the method on a deallocated instance.

PR #9751 have mitigated this by adding a fail-safe, but I think the culprint is the weak reference, which this PR fixes.

Tested on iOS only, since it doesn't touch Android.
Closes #10147

Differential Revision: D3938763

fbshipit-source-id: 7389d4ae7a98926014401a1fe0cbbdcdd5ee6a01
pfeiffer added a commit to pfeiffer/react-native that referenced this issue Oct 12, 2016
@pfeiffer
Use strong reference in RCTImageLoader
a6c06ef 
Summary:
This PR is related to the multitude of crashes (facebook#10016, facebook#9751, facebook#9882).

From my understanding, we should be using a strong reference when calling `decodeImageData` or we could be calling the method on a deallocated instance.

PR facebook#9751 have mitigated this by adding a fail-safe, but I think the culprint is the weak reference, which this PR fixes.

Tested on iOS only, since it doesn't touch Android.
Closes facebook#10147

Differential Revision: D3938763

fbshipit-source-id: 7389d4ae7a98926014401a1fe0cbbdcdd5ee6a01
pfeiffer pushed a commit to pfeiffer/react-native that referenced this issue Oct 12, 2016
@tobycox @pfeiffer
FIX facebook#9751 - Cancelling offscreen image loads causing crashes
c108341 
Summary:
This fixes facebook#9751

We were seeing the same issues as the original reporter.

I'm not an expert on this code, so apologies if it's naive, but we haven't seen the crash since making this change.

From my understanding of it, it seems like the `cancelLoad` block was being released before the block returned by `_loadImageOrDataWithURLRequest:` was called.
This PR checks the `cancelled` flag before calling `cancelLoad()`.
Closes facebook#10016

Differential Revision: D3902723

Pulled By: javache

fbshipit-source-id: 75cd115e28694105c6fc29469986998ca0d4cd09
@janmonschke
Copy link
Contributor

This is still happening for us on 0.35.0

@facebook facebook locked as resolved and limited conversation to collaborators May 24, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Jul 19, 2018
This issue was closed.
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Assignees
No one assigned
Labels
Resolution: Locked This issue was locked by the bot.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

FIX #9751 - Cancelling offscreen image loads causing crashes tobycox/react-native
4 participants
@janmonschke @lprhodes @ymmuse @react-native-bot