Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

UITableView reloadData crashes on reappearance in iOS 11

Update: In my view, the question is still relevant and so I am marking a potential design flaw that I had in my code. I was calling the asynchronous data population method in viewWillAppear: of VC1 which is NEVER a good place to populate data and to reload a table view unless everything is serialized in the main thread. There are always potential execution points in your code when you must reload you table view and viewWillAppear is not one of them. I was always reloading table view data source in VC1 viewWillAppear when returning from VC2. But an ideal design could have used an unwind segue from VC2 and repopulate the data source upon its preparation (prepareForSegue) right from VC2, only when it was actually required. Unfortunately, it seems like nobody had mentioned it so far :(


I think there are similar questions that have been asked previously. Unfortunately none of them essentially addressed the issue I'm facing.

My problem structure is very simple. I have two view controllers, say VC1 and VC2. In VC1 I show a list of some items in a UITableView, loaded from the database and in VC2 I show the details of the chosen item and let it be edited and saved. And when user returns to VC1 from VC2 I must repopulate the datasource and reload the table. Both VC1 and VC2 are embedded in a UINavigationController.

Sounds very trivial and indeed it is, till I do everything in the UI thread. The problem is loading the list in VC1 is somewhat time consuming. So I have to delegate the heavy-lifting of data loading task to some background worker thread and reload the table on main thread only when data load completes to give a smooth UI experience. So my initial construct was something similar to the following:

-(void)viewWillAppear:(BOOL)animated {
    [super viewWillAppear:animated];
    dispatch_async(self.application.commonWorkerQueue, ^{
        [self populateData]; //populate datasource
        dispatch_async(dispatch_get_main_queue(), ^{
            [self.tableView reloadData]; //reload table view
        });
    });
}

This was very much functional until iOS10 from when UITableView stopped immediate rendering through reloadData and started to treat reloadData just as a registration request to reload the UITableView in some subsequent iteration of the run-loop. So I found that my app started to occasionally crash if [self.tableView reloadData] hadn't completed before a subsequent call to [self populateData] and that was very obvious since [self populateData] isn't thread-safe anymore and if datasource changes before the completion of reloadData it is very likely to crash the app. So I tried adding a semaphore to make [self populateData] thread-safe and I found that it was working great. My subsequent construct was something similar to the following:

-(void)viewWillAppear:(BOOL)animated {
    [super viewWillAppear:animated];
    dispatch_async(self.application.commonWorkerQueue, ^{
        [self populateData]; //populate datasource
        dispatch_async(dispatch_get_main_queue(), ^{
            [self.tableView reloadData]; //reload table view
            dispatch_async(dispatch_get_main_queue(), ^{
                dispatch_semaphore_signal(self.datasourceSyncSemaphore); //let the app know that it is free to repopulate datasource again
            });
        });
        dispatch_semaphore_wait(self.datasourceSyncSemaphore, DISPATCH_TIME_FOREVER); //wait on a semaphore so that datasource repopulation is blocked until tableView reloading completes
    });
}

Unfortunately, this construct also broke since iOS11 when I scroll down through UITableView in VC1, select an item that brings up VC2 and then come back to VC1. It again calls viewWillAppear: of VC1 that in turn tries to repopulate the datasource through [self populateData]. But the crashed stack-trace shows that the UITableView had already started to recreate its cells from scratch and calling tableView:cellForRowAtIndexPath: method for some reason, even before viewWillAppear:, where my datasource is being repopulated in background and it is in some inconsistent state. Eventually the application crashes. And most surprisingly this is happening only when I had selected a bottom row that was not on screen, initially. Following is the stack-trace during the crash:

crashed stack trace

I know everything would run fine if I call both the methods from the main thread, like this:

-(void)viewWillAppear:(BOOL)animated {
    [super viewWillAppear:animated];
    [self populateData]; //populate datasource
    [self.tableView reloadData]; //reload table view
}

But that is not something that is expected for a good user experience. I feel the issue happens since UITableView is trying to fetch the offscreen top rows on reappearance, when scrolled down. But unfortunately after understanding so many damn things I could hardly sort it out.

I would really like the experts of this site to help me out of the situation or show me some way around. Thanks a loads in advance!

PS: self.application.commonWorkerQueue is serial dispatch queue running in the background in this context.

like image 763
Ayan Sengupta Avatar asked Oct 14 '17 07:10

Ayan Sengupta


2 Answers

You should split your populateData function. Lets say for example into fetchDatabaseRows and populateDataWithRows. The fetchDatabaseRows should retrieve the rows into memory in its own thread and a new data structure. When the IO part is done, then you should call populateDataWithRows (and then reloadData) in the UI thread. populateDataWithRows should modify the collections used by the TableView.

like image 183
Rick Avatar answered Oct 08 '22 21:10

Rick


UIKit runs on main thread. All UI updates must be on main thread only. There is no race condition if updates to data source happens on main thread only.

Important to understand is that you need to protect data. So if you are using semaphore or mutex or anything like this construct is always:

  1. claim the resource for me. (ex: mutex.lock())
  2. do the processing
  3. unlock the resource (ex: mutex.unlock())

Thing is, that because UI thread is for UI and background thread is used for processing you can not lock shared data source, because you would lock UI thread as well. Main thread would wait for unlock from background thread. So this construct is big NO-NO. That means your populateData() function must create copy of data in the background while UI is using its own copy on main thread. When data are ready, just move the update into main thread (no need for semaphore or mutex)

dispatch_async(dispatch_get_main_queue(), ^{
            //update datasource for table view here
            //call reload data
        });

Another thing: viewWillAppear is not the place to do this update. Because you have navigation where you push your detail, you may do the swipe to dismiss, and in the midle just change your mind and stay in detail. However, vc1 viewWillAppear will be called. Apple should rename that method to "viewWillAppearMaybe" :). So right thing to do is to create a protocol, define method that will be called and use delegation to call the update function just once. This will not cause crash bug, but why to call update more than once? Also, why you are fetching all items, if only one has changed? I would update just 1 item.

One more: You are probably creating reference cycle. Be careful when using self in blocks.

Your first example would be almost good if it looked like this:

-(void)viewWillAppear:(BOOL)animated {
  [super viewWillAppear:animated];
  dispatch_async(self.application.commonWorkerQueue, ^{
    NSArray* newData = [self populateData]; //this creates new array, don't touch tableView data source here!
    dispatch_async(dispatch_get_main_queue(), ^{
        self.tableItems = newData; //replace old array with new array
        [self.tableView reloadData]; //reload
    });
  });
}

(self.tableItems is NSArray, simple data source for tableView as an example of data source)

like image 40
Juraj Antas Avatar answered Oct 08 '22 20:10

Juraj Antas