Skip to content

Commit

Permalink
iOS: Fix dequeueTasks crash in image loader
Browse files Browse the repository at this point in the history
Summary:
This fixes a crash occurring [on this line](https://github.com/facebook/react-native/blob/f9f32eb426b5385e199ef6e1d2b610dfe20e60e7/Libraries/Image/RCTImageLoader.m#L253). It was reported in a comment in facebook#10433.

The problem is that `task` is deallocated at this point and is unsafe to use. Removing it from `_pendingTasks` dropped its ref count to 0. [The ARC docs](http://clang.llvm.org/docs/AutomaticReferenceCounting.html#fast-enumeration-iteration-variables) state that, by default, loop variables in fast enumeration loops are not retained. That's why `task`'s ref count is 0.

It's likely we ran into this bug because the code disobeyed the [reverseObjectEnumerator docs](https://developer.apple.com/reference/foundation/nsarray/1415734-reverseobjectenumerator) which state that "you must not modify the array during enumeration". The default retention policy for fast enumeration seems to assume you follow this.

To fix this bug and avoid other potential pitfalls, the code now follows the docs and `_pendingTa
Closes facebook#11296

Differential Revision: D4277167

Pulled By: javache

fbshipit-source-id: 1211b32935bab7f4080dc00b88d85348786e859a
  • Loading branch information
rigdern authored and gabceb committed Dec 19, 2016
1 parent be5c93c commit e768447
Showing 1 changed file with 13 additions and 2 deletions.
15 changes: 13 additions & 2 deletions Libraries/Image/RCTImageLoader.m
Original file line number Diff line number Diff line change
Expand Up @@ -237,10 +237,14 @@ - (void)dequeueTasks
{
dispatch_async(_URLRequestQueue, ^{
// Remove completed tasks
NSMutableArray *tasksToRemove = nil;
for (RCTNetworkTask *task in self->_pendingTasks.reverseObjectEnumerator) {
switch (task.status) {
case RCTNetworkTaskFinished:
[self->_pendingTasks removeObject:task];
if (!tasksToRemove) {
tasksToRemove = [NSMutableArray new];
}
[tasksToRemove addObject:task];
self->_activeTasks--;
break;
case RCTNetworkTaskPending:
Expand All @@ -249,13 +253,20 @@ - (void)dequeueTasks
// Check task isn't "stuck"
if (task.requestToken == nil) {
RCTLogWarn(@"Task orphaned for request %@", task.request);
[self->_pendingTasks removeObject:task];
if (!tasksToRemove) {
tasksToRemove = [NSMutableArray new];
}
[tasksToRemove addObject:task];
self->_activeTasks--;
[task cancel];
}
break;
}
}

if (tasksToRemove) {
[self->_pendingTasks removeObjectsInArray:tasksToRemove];
}

// Start queued decode
NSInteger activeDecodes = self->_scheduledDecodes - self->_pendingDecodes.count;
Expand Down

0 comments on commit e768447

Please sign in to comment.