Using GCD with table views causing intermittent assert

Question

I am using a synchronous GCD queue to control access to an NSArray and ensure that the array isn't mutated while it's being enumerated or being read from. This works great most of the time, however, I'm receiving an intermittent assertion which only happens randomly after the app has been backgrounded for a while.

My code:

dispatch_async(synchronous_queue, ^{
  // Update the data source and insert the new items
  NSUInteger currentItemsArraySize = originalItems.count;
  [originalItems addObjectsFromArray:newItemsFromCache];
  NSMutableArray *indexPathArray = [NSMutableArray arrayWithCapacity:newItemsSize];
  for (NSUInteger i = currentItemsArraySize; i < (currentItemsArraySize + newItemsSize); i++) {
    [indexPathArray addObject:[NSIndexPath indexPathForRow:i inSection:0]];
  }

  dispatch_async(dispatch_get_main_queue(), ^{
    [self beginUpdates];
    [self insertRowsAtIndexPaths:indexPathArray withRowAnimation:UITableViewRowAnimationNone];
    [self endUpdates];
  });
});

The assertion:

2014-05-09 15:31:03.832 MyApp[8650:60b] * Terminating app due to uncaught exception 'NSInternalInconsistencyException', reason: 'Invalid update: invalid number of rows in section 0. The number of rows contained in an existing section after the update (90) must be equal to the number of rows contained in that section before the update (90), plus or minus the number of rows inserted or deleted from that section (10 inserted, 0 deleted) and plus or minus the number of rows moved into or out of that section (0 moved in, 0 moved out).' * First throw call stack: (0x184ae709c 0x190a65d78 0x184ae6f5c 0x185617194 0x187bb11c4 0x1001204d4 0x191034420 0x1910343e0 0x19103756c 0x184aa6d64 0x184aa50a4 0x1849e5b38 0x18a40b830 0x187a240e8 0x1000ee63c 0x19104faa0) libc++abi.dylib: terminating with uncaught exception of type NSException

Typically, this assertion would be because I didn't update my data source before calling insertRowsAtIndexPaths: but the strange thing is that the assertion seems to think the table update has already been performed even though all I've done is update the table data source array at this point.

For example, when I look at the indexPathArray, it shows me that I'm going to attempt inserting rows 80 through 89, but the assertion says I already have 90 items in the data source which doesn't make sense.

(lldb) po indexPathArray
<__NSArrayM 0x17044ac20>(
<NSIndexPath: 0xc000000000280016> {length = 2, path = 0 - 80},
<NSIndexPath: 0xc000000000288016> {length = 2, path = 0 - 81},
<NSIndexPath: 0xc000000000290016> {length = 2, path = 0 - 82},
<NSIndexPath: 0xc000000000298016> {length = 2, path = 0 - 83},
<NSIndexPath: 0xc0000000002a0016> {length = 2, path = 0 - 84},
<NSIndexPath: 0xc0000000002a8016> {length = 2, path = 0 - 85},
<NSIndexPath: 0xc0000000002b0016> {length = 2, path = 0 - 86},
<NSIndexPath: 0xc0000000002b8016> {length = 2, path = 0 - 87},
<NSIndexPath: 0xc0000000002c0016> {length = 2, path = 0 - 88},
<NSIndexPath: 0xc0000000002c8016> {length = 2, path = 0 - 89}
)

What could be causing this to only happen randomly and how can I prevent this?

---

Edit: I'm going to try rewriting it per this http://blog.csdn.net/chuanyituoku/article/details/17288991 which should essentially do what I'm wanting (ensure the data source isn't mutated while being enumerated) and fix the race conditions Michael described.

- (NSMutableArray *)objects {
  __block NSMutableArray *syncObjects;
  dispatch_sync(synchronous_queue, ^{
    syncObjects = _objects
  });
  return syncObjects;
}

- (void)setObjects:(NSMutableArray *)objects {
  dispatch_barrier_async(synchronous_queue, ^{
    _objects = objects;
  });
}

Answer 1

When you insert rows to a table view (or delete rows from a table view), you have to make sure that subsequent calls to the table view datasource methods provide a consistent worldview to the table view.

E.g. when the table view has 4 rows, and you insert 2 rows, a subsequent call to -numberOfRowsInTableView: must return 6, and the -tableView:cellForRowAtIndexPath: method should return the new data for the two newly inserted rows.

So far so good...

Now, there are race conditions in your code.

First race condition: you access the iVar originalItems from different threads.

Second race condition: after you update the originalItems array, but before the synchronized call on the main thread to -insertRowsAtIndexPaths:withRowAnimation:, the table view may call datasource methods. This is causing the crash. When you do all the updating stuff on the main thread, the crash will go away...

Possible solution

I would do the updating on the main thread only! Therefore, you could create a method named -addItems: that adds new rows to the tableview and which can be called from any thread. There will be no performance penalty if you do this. If the calculation of the newItems array is expensive, you may still do it in another thread, but the actual update must be done on the main thread. Additionally, you can put a synchronized-block around every access to the originalItems array, so that you can have read-only access from other threads.

// you can call this method from any thread
- (void)addItems:(NSArray *)newItems
{
    // Ensure we are on the main thread
    if(![NSThread isMainThread]) {
        newItems = [newItems copy];
        dispatch_async(dispatch_get_main_queue(), ^{
            [self addItems:newItems];
        });
        return;
    }

    // We are on the main thread...

    // Update the data source and insert the new items
    NSMutableArray *indexPathArray;
    @synchronized(self) {
        NSUInteger currentItemsArraySize = originalItems.count;
        [originalItems addObjectsFromArray:newItems];
        indexPathArray = [NSMutableArray new];
        for (NSUInteger i = currentItemsArraySize; i < (currentItemsArraySize + newItemsSize); i++) {
            [indexPathArray addObject:[NSIndexPath indexPathForRow:i inSection:0]];
        }
    }

    [self beginUpdates];
    [self insertRowsAtIndexPaths:indexPathArray withRowAnimation:UITableViewRowAnimationNone];
    [self endUpdates];
}