AFMMRecordResponseSerializer and Core Data threading

[jonbrooks]

i have encountered a Core Data threading-related crash in my app where the root of the problem is that AFMMRecordResponseSerializer's responseObjectForResponse:data:error: is not being called on the main thread, like I believed it was. (See AFHTTPRequestOperation.m, line 127 for reference.)

I am creating the AFMMRecordResponseSerializer instance on the main thread, passing in the main thread's ManagedObjectContext as the parameter. Because the response is serialized on a different thread (and not known at the time the serializer is created) using this context, Core Data actions on the main thread can cause crashes.

I am not sure how to address this problem. It seems AFMMRecordResponseSerializer's responseObjectForResponse:data:error: is designed to be adding objects to the context that's passed in, but since this work actually happens on a different thread, this would mean this is the wrong context to be doing this work.

Am I misunderstanding something here, or is this a general problem with AFMMRecordResponseSerializer? I have not yet tried to reproduce the problem in a simple project.

[jonbrooks]

I can reproduce the problem in the foursquare test app with the following code in MMAppDelegate.m (The problem manifests as either a crash or a deadlock)

- (BOOL)application:(UIApplication *)application didFinishLaunchingWithOptions:(NSDictionary *)launchOptions {
    // AFMMRecordSessionManagerServer Example
    MMFoursquareSessionManager *serverClientManager = [MMFoursquareSessionManager serverClient];

    [AFMMRecordSessionManagerServer registerAFHTTPSessionManager:serverClientManager];
    [FSRecord registerServerClass:[AFMMRecordSessionManagerServer class]];

    // AFMMRecordResponseSerializer Example
    MMFoursquareSessionManager *sessionManager = [MMFoursquareSessionManager sharedClient];

    NSManagedObjectContext *context = [[MMDataManager sharedDataManager] managedObjectContext];
    AFHTTPResponseSerializer *HTTPResponseSerializer = [AFJSONResponseSerializer serializer];

    AFMMRecordResponseSerializationMapper *mapper = [[AFMMRecordResponseSerializationMapper alloc] init];
    [mapper registerEntityName:@"Venue" forEndpointPathComponent:@"venues/search?"];

    AFMMRecordResponseSerializer *serializer =
        [AFMMRecordResponseSerializer serializerWithManagedObjectContext:context
                                                responseObjectSerializer:HTTPResponseSerializer
                                                            entityMapper:mapper];

    sessionManager.responseSerializer = serializer;

    [NSTimer scheduledTimerWithTimeInterval:0.001 target:self selector:@selector(fetchAll:) userInfo:nil repeats:YES];

    return YES;
}

- (void)fetchAll:(NSTimer*)timer {
    NSManagedObjectContext *context = [[MMDataManager sharedDataManager] managedObjectContext];    
    NSFetchRequest *request = [[NSFetchRequest alloc] init];
    NSError *error;
    request.entity = [NSEntityDescription entityForName:@"Venue" inManagedObjectContext:context];
    NSArray *result = [context executeFetchRequest:request error:&error];
}

[jonbrooks]

BTW-the problem only seems to happen on device. (I'm on iPhone 5s, OS 7.1)

[cnstoll]

Damn, thats a weird one. The frustrating part is that we're doing the right thing by handling the background context with a performBlockAndWait. But I think the issue may be that we're not doing the same thing with the primary context. Check out the lines below:

    for (NSManagedObjectID *objectID in objectIDs) {
        [records addObject:[self.context objectWithID:objectID]];
    }

We're calling objectWithID on the primary context (which is a main thread context in your example), but AFNetworking makes no real guarantee on what thread that callback will be on.

I think the best solution is likely to wrap that in a self.context performBlockAndWait method as well...

Do you feel like replacing that for loop with this in your local copy and see if that fixes the race condition?

    __block NSMutableArray *records = [NSMutableArray array];
    NSManagedObjectContext *context = self.context;

    [self.context performBlockAndWait:^{
        for (NSManagedObjectID *objectID in objectIDs) {
            [records addObject:[context objectWithID:objectID]];
        }
    }];

For what its worth, MMRecord.m handles this itself with an internal queue management system. It was my hope that we wouldn't have to fully port that system over to this serializer extension, because the goal was to have MMRecordResponse be relatively self contained to the point that wouldn't be necessary. While I believe this should fix the issue, ultimately I think the real solution is to make MMRecordResponse into a block based API so that the user of that class doesn't have to care about threading at all.

But, thats probably down the road.

Thanks,

• Conrad

[jonbrooks]

I actually tried just the performBlockAndWait change you suggested earlier today and it did not fix the problem. After looking at it some more, I came to the conclusion that the MMRecordResponse itself was doing things with the primary context that would be problematic on the wrong thread. What ended up fixing it was to initialize the MMRecordResponse with the background context, so now the RecordResponse is working on the proper context for the current thread. This resulted in blank records until I added a -save: after the NSArray *records = [recordResponse records]; line. I have to confess I don't understand why the save is necessary when it wasn't before.

I'll submit a pull request for your review.