YapDatabase icon indicating copy to clipboard operation
YapDatabase copied to clipboard

Reproducible crash: attempt to perform an insert and a move to the same index path

Open joelekstrom opened this issue 5 years ago • 2 comments

We're getting this crash quite often when updating a view from changes with view mappings. It's happening (presumably) when there are many updates happening at the same time. It's possible that this crash is due to a bug in UITableView (it also happens in UICollectionView). The crash is reproducible both on simulator and device. The frequency of the crashes seem directly related to how big the changeset is.

attempt to perform an insert and a move to the same index path (<NSIndexPath: 0xbd250b35a3c418f2> {length = 2, path = 4 - 2})

Sometimes the crash will instead be this:

attempt to insert row 3 into section 9, but there are only 2 rows in section 9 after the update

I have made a sample view controller that will quickly reproduce this crash. However, it's not deterministic and will happen on different changesets every time, even if the changes are exactly the same on each run.

The interesting thing is, that it doesn't seem like YapDatabase actually reports a move and insert to the same index path, since then the assert inside yapDatabaseModified: should've fired, which is what leads me to believe that the crash might be in UITable/CollectionView.

Has anyone else ran into this? Could it be a YapDatabase-issue? We're unfortunately getting this a lot, and we might have to turn off animated updates completely.

Possible fix

Ignoring all YapDatabaseViewChangeUpdate seems to completely fix all crashes. Possibly related to this crash also observed here. It appears that reloads simply aren't compatible with other collection updates.

Solution!

When doing many/quick updates, reloads must be performed individually. In yapDatabaseModified:, here's one way to fix all the crashes:

  1. In your objectAtIndexPath:-function, cache the NSIndexPath -> YapCollectionKey-pair
  2. In yapDatabaseModified:
    1. From the notifications, filter out all YapDatabaseRowChanges with of type YapDatabaseViewChangeUpdate
    2. Call reloadRowsAtIndexPaths for these updates. The table view will call cellForRowAt:.. which should return an object according to the cache mentioned in 1.
    3. Clear the cache
    4. Update the viewMappings to grab the changes the normal way
    5. Perform all the batch updates the normal way, but skip all YapDatabaseViewChangeUpdate!

Implementation example here

The cache is needed because when you call reloadItemsAtIndexPaths:, the viewConnection will be at the latest database snapshot, but the viewMappings will not, so it seems that the rowids doesn't match. The cache makes sure that we always fetch the correct object because we have associated index paths with a collection/key-pair.

Reproducing view controller

Just add it as the initial view controller of an empty project.

#import "ViewController.h"
#import "AppDelegate.h"
#import <YapDatabase/YapDatabaseAutoView.h>

@interface ViewController ()
@property (nonatomic, strong) YapDatabase *database;
@property (nonatomic, strong) YapDatabaseConnection *connection;
@property (nonatomic, strong) YapDatabaseConnection *writeConnection;
@property (nonatomic, strong) YapDatabaseViewMappings *mappings;
@end

@implementation ViewController

#define RND(MAX) MAX>0?(rand()%MAX):0

- (void)loadDatabase
{
    NSURL *library = [[[NSFileManager defaultManager] URLsForDirectory:NSLibraryDirectory inDomains:NSUserDomainMask] firstObject];
    NSURL *databaseURL = [library URLByAppendingPathComponent:@"/database.sqlite"];
    self.database = [[YapDatabase alloc] initWithPath:databaseURL.path];
    [[self.database newConnection] readWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) {
        [transaction removeAllObjectsInAllCollections];
    }];
}

- (void)viewDidLoad {
    [super viewDidLoad];
    srand(10); // Used to reproduce changesets
    [self loadDatabase];
    self.connection = self.database.newConnection;
    self.writeConnection = self.database.newConnection;
    [self registerView];
    [self setupViewMappings];
    [self.tableView reloadData];
    [NSNotificationCenter.defaultCenter addObserver:self selector:@selector(yapDatabaseModified:) name:YapDatabaseModifiedNotification object:nil];
    [NSTimer scheduledTimerWithTimeInterval:1 repeats:YES block:^(NSTimer * _Nonnull timer) {
        [self changeData];
    }];
}

- (void)registerView {
    if ([self.database registeredExtension:@"view"]) {
        return;
    }
    YapDatabaseView *view = [[YapDatabaseAutoView alloc] initWithGrouping:[YapDatabaseViewGrouping withObjectBlock:^NSString *(YapDatabaseReadTransaction *transaction, NSString *collection, NSString *key, NSDictionary *object) {
        return [object[@"section"] stringValue];
    }] sorting:[YapDatabaseViewSorting withObjectBlock:^NSComparisonResult(YapDatabaseReadTransaction *transaction, NSString *group, NSString *collection1, NSString *key1, NSDictionary *object1, NSString *collection2, NSString *key2, NSDictionary *object2) {
        return [object1[@"value"] compare:object2[@"value"]];
    }]];
    [self.database registerExtension:view withName:@"view"];
}

- (void)setupViewMappings {
    NSMutableArray *groups = [NSMutableArray new];
    for (int i = 0; i < 10; ++i) {
        [groups addObject:@(i).stringValue];
    }
    self.mappings = [YapDatabaseViewMappings mappingsWithGroups:groups view:@"view"];
    self.mappings.isDynamicSectionForAllGroups = YES;
    [self.connection beginLongLivedReadTransaction];
    [self.connection readWithBlock:^(YapDatabaseReadTransaction *transaction) {
        [self.mappings updateWithTransaction:transaction];
    }];
}

- (void)yapDatabaseModified:(NSNotification *)notification {
    NSArray *notifications = [self.connection beginLongLivedReadTransaction];
    NSArray *sectionChanges = nil;
    NSArray *rowChanges = nil;
    [[self.connection ext:@"view"] getSectionChanges:&sectionChanges
                                          rowChanges:&rowChanges
                                    forNotifications:notifications
                                        withMappings:self.mappings];
    if ([sectionChanges count] == 0 & [rowChanges count] == 0) {
        return;
    }

    [self.tableView performBatchUpdates:^{
        for (YapDatabaseViewSectionChange *sectionChange in sectionChanges) {
            switch (sectionChange.type) {
                case YapDatabaseViewChangeDelete:
                    [self.tableView deleteSections:[NSIndexSet indexSetWithIndex:sectionChange.index] withRowAnimation:UITableViewRowAnimationAutomatic];
                    break;
                case YapDatabaseViewChangeInsert:
                    [self.tableView insertSections:[NSIndexSet indexSetWithIndex:sectionChange.index] withRowAnimation:UITableViewRowAnimationAutomatic];
                    break;
                default:
                    break;
            }
        }

        for (YapDatabaseViewRowChange *rowChange in rowChanges) {
            switch (rowChange.type) {
                case YapDatabaseViewChangeDelete:
                    [self.tableView deleteRowsAtIndexPaths:@[rowChange.indexPath] withRowAnimation:UITableViewRowAnimationAutomatic];
                    break;
                case YapDatabaseViewChangeInsert:
                    [self.tableView insertRowsAtIndexPaths:@[rowChange.newIndexPath] withRowAnimation:UITableViewRowAnimationAutomatic];
                    break;
                case YapDatabaseViewChangeMove:
                    for (YapDatabaseViewRowChange *c in rowChanges) {
                        if (c.type == YapDatabaseViewChangeInsert) {
                            NSAssert(![c.newIndexPath isEqual:rowChange.newIndexPath], @"Move and insert to same index path");
                        }
                    }
                    [self.tableView moveRowAtIndexPath:rowChange.indexPath toIndexPath:rowChange.newIndexPath];
                    break;
                case YapDatabaseViewChangeUpdate :
                    [self.tableView reloadRowsAtIndexPaths:@[rowChange.indexPath] withRowAnimation:UITableViewRowAnimationNone];
                    break;
            }
        }
    } completion:nil];
}

- (NSInteger)numberOfSectionsInTableView:(UITableView *)tableView {
    return self.mappings.numberOfSections;
}

- (NSInteger)tableView:(UITableView *)tableView numberOfRowsInSection:(NSInteger)section {
    return [self.mappings numberOfItemsInSection:section];
}

- (NSString *)tableView:(UITableView *)tableView titleForHeaderInSection:(NSInteger)section {
    return [self.mappings groupForSection:section];
}

- (UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(NSIndexPath *)indexPath {
    UITableViewCell *cell = [tableView dequeueReusableCellWithIdentifier:@"cell"];
    __block NSDictionary *obj = nil;
    [self.connection readWithBlock:^(YapDatabaseReadTransaction *transaction) {
        obj = [(YapDatabaseViewTransaction *)[transaction ext:@"view"] objectAtIndexPath:indexPath withMappings:self.mappings];
    }];
    cell.textLabel.text = [obj[@"value"] stringValue];
    return cell;
}

- (void)changeData {
    [self.writeConnection asyncReadWriteWithBlock:^(YapDatabaseReadWriteTransaction *transaction) {
        // Delete some keys
        {
            NSArray *existingKeys = [transaction allKeysInCollection:@"collection"];
            NSUInteger numberOfKeysToDelete = RND(existingKeys.count);
            NSMutableSet *keysToDelete = [NSMutableSet new];
            for (int i = 0; i < numberOfKeysToDelete; ++i) {
                [keysToDelete addObject:existingKeys[RND(existingKeys.count)]];
            }
            [transaction removeObjectsForKeys:keysToDelete.allObjects inCollection:@"collection"];
        }

        // Move some objects around
        {
            NSArray *existingKeys = [transaction allKeysInCollection:@"collection"];
            NSUInteger numberOfKeysToMove = RND(existingKeys.count);
            for (int i = 0; i < numberOfKeysToMove; ++i) {
                NSString *key = existingKeys[RND(existingKeys.count)];
                NSMutableDictionary *obj = [[transaction objectForKey:key inCollection:@"collection"] mutableCopy];
                obj[@"section"] = @(RND(10));
                obj[@"value"] = @(RND(100));
                [transaction setObject:[obj copy] forKey:key inCollection:@"collection"];
            }
        }

        // Insert objects so we have between 0-80 items in total
        {
            NSUInteger count = [transaction numberOfKeysInCollection:@"collection"];
            NSUInteger target = MAX(RND(80), count);
            for (NSUInteger i = count; i < target; ++i) {
                [transaction setObject:@{@"section": @(RND(10)), @"value": @(RND(100))} forKey:[NSUUID UUID].UUIDString inCollection:@"collection"];
            }
        }
    }];
}

@end

joelekstrom avatar May 28 '19 10:05 joelekstrom

Here's the fix discussed above:

@interface MyClass
@property (nonatomic, strong) NSMutableDictionary<NSIndexPath *, YapCollectionKey *> *cache;
@end

@implementation MyClass

- (id)objectAtIndexPath:(NSIndexPath *)indexPath {
    __block id object = nil;
    [self.connection readWithBlock:^(YapDatabaseReadTransaction *transaction) {
        YapCollectionKey *collectionKey = self.cache[indexPath];
        if (collectionKey == nil) {
            NSString *key = nil;
            NSString *collection = nil;
            [[transaction extension:self.mappings.view] getKey:&key collection:&collection atIndexPath:indexPath withMappings:self.mappings];
            collectionKey = [[YapCollectionKey alloc] initWithCollection:collection key:key];
            self.cache[indexPath] = collectionKey;
        }
        object = [transaction objectForKey:collectionKey.key inCollection:collectionKey.collection];
    }];
    return object;
}

- (void)yapDatabaseModified:(NSNotification *)notification {
    NSArray *notifications = [self.connection beginLongLivedReadTransaction];

    // Perform reloads independently. Fixes a crash in UITableView and UICollectionView
    // when doing reloads at the same times as other updates
    NSSet<YapCollectionKey *> *collectionKeys = [self updatedCollectionKeysInNotifications:notifications];
    NSSet<NSIndexPath *> *updatedIndexPaths = [self.cache keysOfEntriesPassingTest:^BOOL(NSIndexPath *indexPath, YapCollectionKey *collectionKey, BOOL *stop) {
        return [collectionKeys containsObject:collectionKey];
    }];
    if (updatedIndexPaths.count > 0) [self.view reloadItemsAtIndexPaths:updatedIndexPaths.allObjects];

    // Clear the cache. Now we will perform the actual inserts/moves etc so index paths will change.
    [self.cache removeAllObjects];

    NSArray *sectionChanges = nil;
    NSArray *rowChanges = nil;

    [[self.connection ext:@"view"] getSectionChanges:&sectionChanges
                                          rowChanges:&rowChanges
                                    forNotifications:notifications
                                        withMappings:self.mappings];

    if ([sectionChanges count] == 0 & [rowChanges count] == 0) {
        return;
    }

    [self.tableView performBatchUpdates:^{
        for (YapDatabaseViewSectionChange *sectionChange in sectionChanges) {
            switch (sectionChange.type) {
                case YapDatabaseViewChangeDelete:
                    [self.tableView deleteSections:[NSIndexSet indexSetWithIndex:sectionChange.index] withRowAnimation:UITableViewRowAnimationAutomatic];
                    break;
                case YapDatabaseViewChangeInsert:
                    [self.tableView insertSections:[NSIndexSet indexSetWithIndex:sectionChange.index] withRowAnimation:UITableViewRowAnimationAutomatic];
                    break;
                default:
                    break;
            }
        }

        for (YapDatabaseViewRowChange *rowChange in rowChanges) {
            switch (rowChange.type) {
                case YapDatabaseViewChangeDelete:
                    [self.tableView deleteRowsAtIndexPaths:@[rowChange.indexPath] withRowAnimation:UITableViewRowAnimationAutomatic];
                    break;
                case YapDatabaseViewChangeInsert:
                    [self.tableView insertRowsAtIndexPaths:@[rowChange.newIndexPath] withRowAnimation:UITableViewRowAnimationAutomatic];
                    break;
                case YapDatabaseViewChangeMove:
                    for (YapDatabaseViewRowChange *c in rowChanges) {
                        if (c.type == YapDatabaseViewChangeInsert) {
                            NSAssert(![c.newIndexPath isEqual:rowChange.newIndexPath], @"Move and insert to same index path");
                        }
                    }
                    [self.tableView moveRowAtIndexPath:rowChange.indexPath toIndexPath:rowChange.newIndexPath];
                    break;
                case YapDatabaseViewChangeUpdate:
                    // All updates were handled individually above, ignore
                    break;
            }
        }
    } completion:nil];
}

- (NSSet<YapCollectionKey *> *)updatedCollectionKeysInNotifications:(NSArray<NSNotification *> *)notifications
{
    NSMutableSet<YapCollectionKey *> *collectionKeys = [NSMutableSet new];
    for (NSNotification *notification in notifications) {
        for (NSDictionary *extension in [notification.userInfo[YapDatabaseExtensionsKey] allValues]) {
            for (id change in extension[YapDatabaseViewChangesKey]) {
                if ([change isKindOfClass:YapDatabaseViewRowChange.class] && [(YapDatabaseViewRowChange *)change type] == YapDatabaseViewChangeUpdate) {
                    [collectionKeys addObject:[(YapDatabaseViewRowChange *)change collectionKey]];
                }
            }
        }
    }
    return collectionKeys;
}

@end 

joelekstrom avatar May 28 '19 20:05 joelekstrom

Added an updated and simpler solution.

joelekstrom avatar Jun 12 '19 12:06 joelekstrom