Core Data threading and lock contention issues

Question

I'm currently writing the sync engine of my iOS app. One of the methods I'm writing is a reload data function, in which the app re-downloads the user's data as well as all of their photos. This is an expensive operation (time-wise), so I created an NSOperation subclass, SSReloadDataOperation. It downloads the data, gets the currentUser entity, removes all existing photos from that currentUser, and repopulates it.

However, even though I thought this was thread-safe, sometimes while the operation is running and -currentUser is accessed from somewhere else, the app crashes, presumably while trying to fetch it. Other times, the UI sometimes just freezes, and pausing in the debugger shows it always stalls at a -currentUser NSFetchRequest execution call.

How do I make this operation thread-safe and atomic, such that I can download and repopulate without blocking the main UI thread, and still have -currentUser be accessible? Is there something I'm missing in terms of using locks or architecture? Thank you!

Code:

- (void)main
{
  // Download the photo data
  [[SyncEngine defaultEngine] getUserPhotosWithCompletionBlock:^(NSMutableArray *photos)
  {
     if (photos)
     {
         // Create a new NSManagedObjectContext for this operation

         SSAppDelegate* appDelegate = [[UIApplication sharedApplication] delegate];
         NSManagedObjectContext* localContext = [[NSManagedObjectContext alloc] init];
         [localContext setPersistentStoreCoordinator:[[appDelegate managedObjectContext] persistentStoreCoordinator]];

         NSNotificationCenter* notificationCenter = [NSNotificationCenter defaultCenter];
         [notificationCenter addObserver:self
                                selector:@selector(mergeChanges:)
                                    name:NSManagedObjectContextDidSaveNotification
                                  object:localContext];

         NSError* error;
         NSFetchRequest* request = [[[SyncEngine defaultEngine] managedObjectModel] fetchRequestFromTemplateWithName:@"CurrentUser" substitutionVariables:[[NSDictionary alloc] init]];
         User* currentUser = [[localContext executeFetchRequest:request error:&error] objectAtIndex:0];

         // Remove the old data
         [currentUser setPhotos:[[NSSet alloc] init]];

         // Iterate through photo data, repopulate
         for (Photo* photo in photos) {
             [currentUser addPhotosObject:photo];
         }

         if (! [localContext save:&error]) {
             NSLog(@"Error saving: %@", error);
         }

         NSLog(@"Completed sync!");
     }
  } userId:[[[SyncEngine defaultEngine] currentUser] userId]];
}

-currentUser convenience method, usually called from the main thread.

- (User *)currentUser
{
   NSError* error;
   NSFetchRequest* request = [self.managedObjectModel fetchRequestFromTemplateWithName:@"CurrentUser" substitutionVariables:[[NSDictionary alloc] init]];
    NSArray* result = [self.managedObjectContext executeFetchRequest:request error:&error];
    if ([result count] == 1) {
    return [result objectAtIndex:0];
    }
    return nil;
}

Answer 1

Your problem is these lines, I think;

     NSNotificationCenter* notificationCenter = [NSNotificationCenter defaultCenter];
     [notificationCenter addObserver:self
                            selector:@selector(mergeChanges:)
                                name:NSManagedObjectContextDidSaveNotification
                              object:localContext];

Because notifications fire in the thread in which the notification happens, your -mergeChanges: method is being called in the background thread. I’m assuming it accesses self.managedObjectContext and adds the objects to it, but it’s doing it from a background thread. So, yah, it’ll crash or hang.

You don’t need to register for this notification at all, it looks like, since you only save this context once. You can just wait until you call save, then schedule a merge: on the main thread. Like this:

     if (![localContext save:&error])
         NSLog(@"Error saving: %@", error);

     [[NSOperationQueue mainQueue] addOperationWithBlock:^{
         [self mergeChanges:nil];
     }];

     NSLog(@"Completed sync!");

Answer 2

You are right, this freeze feels like a threading issue.

Because managed objects must be used in the same thread or serial queue where their context is used and where they were created, it is impossible to have a method like currentUser from the example, and make it magically return thread-safe managed object.

But you could pass context as a parameter. The caller will decide in which context to resurrect the user.

- (User *)userWithContext:(NSManagedContext *)context {
    User *user = nil;

    NSError *error;
    NSFetchRequest *request = ...;
    NSArray *userArray = [context executeFetchRequest:request error:&error];
    if (userArray == nil) {
        NSLog(@“Error fetching user: %@“, error);
    } else if ([userArray count] > 0) {
        user = userArray[0];
    }

    return user;
}

And here's other thoughts about your code snippets.

The context you create inside your operation’s main could be a child of your main context. Then, after saving this child, you save the parent (if you want the data to go into the store at this point). When using parent-child relationship you don’t need to subscribe to the did-save notification and merge changes from it.

Technically, you can’t access persistent store coordinator of your main context from operation’s main. Because it is only allowed to work (call any methods) with contexts from one thread or queue. And I bet you create your main context on the main thread.