4

I have a UITableView that sometimes has rapid insertions of new rows. The insertion of the new rows is handled by a notification observer listening for the update notification fired whenever the underlying data changes. I use a @synchronized block around all the data model changes and the actual notification post itself... hoping that each incremental data change (and row insertion) will be handled separately. However, there are times when this still fails. The exception will tell me that it expects 10 rows (based on the count from the data model), it previously had 8 rows, but the update notification only told it to insert a single row (as this is the first of two rapidly fired notifications).

I'm trying to understand how other people tend to handle these types of situations. How do other developers mitigate the problems of having multi-threaded race conditions between two table view update operations? Should I have a more secure lock that controls the update notifications (and why isn't @synchronized doing what it's supposed to)?

Any advice is greatly appreciated! Thanks.

Some pseudo-code:

My model class has a method like this, which gets called by other threads to append new rows to the table view:

- (void)addRow:(NSString *)data
{
    @synchronized(self.arrayOfData)
    {
        NSInteger nextIndex = self.arrayData.count;
        [self.arrayData addObject:data];
        [NSNotificationCenter.defaultCenter postNotificationName:kDataUpdatedNotification object:self userInfo:@{@"insert": @[[NSIndexPath indexPathForRow:nextIndex inSection:0]]}];
    }
}

My controller class has a method like this to accept the kDataUpdatedNotification notification and actually perform the row insertion:

- (void)onDataUpdatedNotification:(NSNotification *)notification
{
    NSDictionary *changes = notification.userInfo;
    [self.tableView insertRowsAtIndexPaths:changes[@"insert"] withRowAnimation:UITableViewRowAnimationBottom];
} 
Mr. T
  • 12,795
  • 5
  • 39
  • 47
  • Can you post some code? – JonahGabriel Jul 16 '13 at 00:08
  • shouldn't you just update the tables array and then call table reload data instead of forcing it this way – rezand Jul 16 '13 at 00:26
  • that is one way to do it, but isn't it more efficient to insert the rows that you're actually changing instead of reloading the entire table? – Mr. T Jul 16 '13 at 00:28
  • i was told/seen on stack other issues and told not to manipulate table directly, you want to manipulate your nsmutable array the table builds its self from and then tell the table "everythings ok, now update" it looks like your code above is kinda the equivalent to removing from a nsmutablearray while enumerating through it. – rezand Jul 16 '13 at 00:31
  • I've hesitated to go down this route because I lose the row-level animation, but at this point, I've just about had enough of this bug, so I'm strongly considering this approach. Let me give it a shot and see if the look+feel is drastically different. Thanks! – Mr. T Jul 16 '13 at 00:34
  • your way may still work i just do not see in the notification where the data is added to your data source, you're just adding to the table so the next move in the table or update the program is confused – rezand Jul 16 '13 at 00:39
  • Another question, are you always calling addRow on the main thread? If not, that could definitely cause some issues. – JonahGabriel Jul 16 '13 at 00:40
  • if you are getting your count from your model you can end up with something like 10 and only be adding your 9th row, so to keep working the way you are with this code you would have to have your own count in your view controller with it's own pace separate from model. – rezand Jul 16 '13 at 00:44
  • @rezand I'm not sure I understand your concern about adding the data to the data source. The purpose of the addRow method is to add the new row's data to the data source. However, the idea of keeping a separate counter in the VC of how many rows/sections it sees is interesting. I might give that a shot – Mr. T Jul 16 '13 at 05:10
  • @Jonah.at.GoDaddy I just tried @TimothyMoose's suggestion to run everything on the main thread and it didn't quite work for me. Perhaps I need to re-architect my code to make it work on the main thread, but `dispatch_async` still had the bug and `dispatch_sync` was too slow. – Mr. T Jul 16 '13 at 05:11
  • @Mr.T sorry about the confusion, I guess the best way to put what I was trying to say is in your onUpdateNotification I just felt like some type of reference for controller class was missing, some sort of count reference or some other variable to keep track of what the controller has verses what the model has but it all depends on the program as a whole as to what your exact needs may be, hope you get this figured out. – rezand Jul 16 '13 at 06:00

1 Answers1

8

You're going to have this problem if you change your data model asynchronously with the main queue because your table view delegate methods are looking at the current state of the data model, which may be ahead of the inserts you've reported to the table view.

UPDATE

One solution is to queue your updates on a private queue and have that queue update your data model on the main queue synchronously (I have not tested this code):

@interface MyModelClass ()
@property (strong, nonatomic) dispatch_queue_t myDispatchQueue;
@end

@implementation MyModelClass

- (dispatch_queue_t)myDispatchQueue
{
    if (_myDispatchQueue == nil) {
        _myDispatchQueue = dispatch_queue_create("myDispatchQueue", NULL);
    }
    return _myDispatchQueue;
}

- (void)addRow:(NSString *)data
{
    dispatch_async(self.myDispatchQueue, ^{
        dispatch_sync(dispatch_get_main_queue(), ^{
            NSInteger nextIndex = self.arrayData.count;
            [self.arrayData addObject:data];
            [NSNotificationCenter.defaultCenter postNotificationName:kDataUpdatedNotification object:self userInfo:@{@"insert": @[[NSIndexPath indexPathForRow:nextIndex inSection:0]]}];
        });
    });
}

The reason you need the intermediate dispatch queue is as follows. In the original solution (below), you get a series of blocks on the main queue that look something like this:

  1. Add row N
  2. Add row N+1
  3. Block posted by table view for row N animation
  4. Block posted by table view for row N+1 animation

In step (3), the animation block is out-of-sync with the table view because (2) happened first, which results in an exception (assertion failure, I think). So, by posting the add row blocks to the main queue synchronously from a private dispatch queue, you get something like the following:

  1. Add row N
  2. Block posted by table view for row N animation
  3. Add row N+1
  4. Block posted by table view for row N+1 animation

without holding up your worker queues.

ORIGINAL Solution still has issues with overlapping animations.

I think you'll be fine if you update your data model on the main queue:

- (void)addRow:(NSString *)data
{
    dispatch_async(dispatch_get_main_queue(), ^{
        NSInteger nextIndex = self.arrayData.count;
        [self.arrayData addObject:data];
        [NSNotificationCenter.defaultCenter postNotificationName:kDataUpdatedNotification object:self userInfo:@{@"insert": @[[NSIndexPath indexPathForRow:nextIndex inSection:0]]}];
    });
}
Timothy Moose
  • 9,895
  • 3
  • 33
  • 44
  • 1
    I think you want dispatch_sync, not dispatch_async. – JonahGabriel Jul 16 '13 at 00:44
  • No, I mean dispatch_async. It posts a block to the specified queue without causing the current thread to wait for that block to complete. – Timothy Moose Jul 16 '13 at 00:46
  • Once you get the data model updates on the main queue, they will happen synchronously with the table view inserts, which also happen on the main queue. – Timothy Moose Jul 16 '13 at 00:53
  • Interesting suggestion, let me give this a shot. I'll report back. Thanks! – Mr. T Jul 16 '13 at 00:59
  • If it doesn't work, try `dispatch_sync` as @Jonah.at.GoDaddy suggested. I realized you may still have issues with overlapping animations (you would likely see a different error) and `dispatch_sync` should solve that. But it could slow down your app depending on what the background threads are doing since they'll have to wait on the main queue. – Timothy Moose Jul 16 '13 at 01:49
  • So `dispatch_async` still results in similar problems. Although it does seem like the exception occurs less frequently. `dispatch_sync` slows down the app quite a bit. I might be doing something inefficiently (there are a lot of calculations that happen in the addRow method) and that's causing the slow down, but I'm not sure this will work. Any other ideas? – Mr. T Jul 16 '13 at 05:08
  • What about the use of dispatch_after inside your dispatch_async functions to add a bit of delay to the table updates? – JonahGabriel Jul 16 '13 at 17:32
  • I think the updated solution will work as is. Did you mean using `dispatch_after` with the original answer? – Timothy Moose Jul 16 '13 at 18:01
  • I was just trying to come up with something to help with the overlapping animation issues. If you put a small delay before you call dispatch_sync, you could help mitigate that problem. – JonahGabriel Jul 16 '13 at 23:06
  • @Jonah.at.GoDaddy Got it. I think the blocks just need to run in the correct order as described in the answer. The duration of the animations themselves aren't relevant. So I don't think inserting a delay is necessary. Hope that makes sense. – Timothy Moose Jul 17 '13 at 17:32
  • 1
    Hey guys, thanks for all of your help. I ended up doing something very similar to Timothy's original suggestion of using the main thread with dispatch_async (without using a separate queue). I needed to re-arch a bunch of code to get it to work. At a high level it's the same as Timothy said, whenever I'm modifying the data, everything that changes (data + UI) needs to be done serially on the main thread. I had some peripheral work that was being done asynchronously. After I changed the design of the code, moved everything to the main thread, the animation race condition was gone. Yay! – Mr. T Jul 17 '13 at 22:33