magicalpanda/MagicalRecord

Resolving `CoreData could not fulfill a fault` best practice

thomassnielsen opened this issue · 14 comments

I keep getting random crash reports from my app saying CoreData could not fulfill a fault. I've been trying to debug this for ages, so I'd love to hear your thoughts on the best way to fix this.

My app is a front end for a REST API. This means it mostly parses and writes JSON, and caches everything locally in Core Data through MagicalRecord.

My observations so far point to a few possible issues:
Whenever I delete an object, there seems to be possible issues with faulting. I suspect this might be because of something pointing to the object after it's been deleted from core data. In particular, I recently implemented a de-duplicator (to clean up after a previous bug I had), and when I deleted a bunch of dupes, the app crashed for a few of my users.

There also seem to be a problem updating objects some times. I'm not sure if this happens because of deleted objects or if there is a problem reading from CD at the time.

My question boils down to:

  1. Are there any good methods of debugging this issue I should be using? I've got code lines for where the crash happens, but it's mostly just a random place where I read or write to Core Data.
  2. Are there any specific measures I should take when deleting from Core Data? Currently I call MR_deleteEntity in a saveWithBlock, should I call MR_saveToPersistentStoreAndWait or try to merge contexts or similar?
  3. Are there any concurrency issues I should be checking out? Like possible issues with writing from multiple simultaneous blocks?
  4. Are there any parts of MagicalRecord I should read and try to understand to better deal with this issue?

If I figure out my issues here I figure I should do a write up of my travels, so others can find them. Googling the error doesn't give me anything useful, just specific fixes for specific cases.

It sounds like you have an NSManagedObjectContext that has invalid NSManagedObjects due to some actions in different contexts.

  1. Have you turned on Core Data concurrency debugging?
  2. When you call MR_deleteEntity inside that save block, are you using a local NSManagedObject? Otherwise that is a concurrency violation. Also you do not need to call MR_saveToPersistentStoreAndWait inside the save block as that will be called after your block finishes execution.
  3. Whenever you use the localContext provided by MagicalRecord, make sure that your objects are in that context by calling NSManagedObject *localObject = [myObject MR_inContext:localContext]. Otherwise, concurrency violation, and then "AllThatIsLeftToUsIsHonor". 😜
  4. The source files 😄 Seriously though, are you doing much with multiple contexts? If so then posting how those contexts interact would be helpful. Also turning on concurrency debugging would be helpful as well.

One last thought (and @tonyarnold correct me if I'm out of line here), I believe the MR team prefers questions like this be posted on StackOverflow as this isn't really an issue with MR but rather a question of implementation.

Thanks a lot, I'll look through each point in turn and report back. When it comes to posting here vs. SO I thought about it, but thinking I might want to update the docs afterwards with some helpful best practices, I figured I could sneak this one in here as an issue.

Ok, CD concurrency debugging is exactly what I needed. It has led me to a few questions though. It seems like I can't run MR_countOfEntitiesWithPredicate on a background queue through dispatch_async(kBgQueue, ^{});. Is this to be expected, or something particular to my implementation? Looking at the source, MR_countOfEntitiesWithPredicate uses MR_contextForCurrentThread, which leads me to believe that it should work, but apparently doesn't for me.

Here's my use case:

+ (NSInteger)badgeCount
{
    NSArray *newStatuses = @[[NSNumber numberWithInteger:PAMOrderItemStatusNew], [NSNumber numberWithInteger:PAMOrderItemStatusPluck]];
    NSNumber *shopId = [[NSUserDefaults standardUserDefaults] objectForKey:@"shopId"];
    Shop *shop = [Shop MR_findFirstByAttribute:@"systemId" withValue:shopId];
    NSPredicate *orderPredicate = [NSPredicate predicateWithFormat:@"SUBQUERY(items, $i, $i.status IN %@ AND $i.shop = %@).@count > 0", newStatuses, shop];
    return [Order MR_countOfEntitiesWithPredicate:orderPredicate];
}

If I call badgeCount from main thread, it works just fine. If I call it inside a dispatch block on a background queue (dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0)), the concurrency debugger barks at me.

MR_contextForCurrentThread is deprecated. Make sure you are using the latest version of MR from the develop branch, or grab it from the releases tab (v2.3beta5). If you have the latest and still see calls to MR_contextForCurrentThread, fix it and submit a pull request 😄 .
So, are you calling +badgeCount from inside the previously mentioned dispatch_async? If so, try creating a context in your +badgeCount method:
NSManagedObjectContext *localContext = [NSManagedObjectContext MR_context];
Then get the order with that context:

__block NSUInteger count = 0;
[localContext performBlockAndWait:^{
    count = [Order MR_countOfEntitiesWithPredicate:orderPredicate inContext:localContext];
}];
return count;

I'm using 2.3b5. There are tons of MR_contextForCurrentThread still in use here: https://github.com/magicalpanda/MagicalRecord/blob/v2.3.0-beta.5/MagicalRecord/Categories/NSManagedObject/NSManagedObject%2BMagicalAggregation.m

To fix this, would I use the same technique as you described above? (creating a new context for each operation?) I'd love to create a pull request, but I'd like to make sure I'm doing it right first 😁

Hmm, I think @tonyarnold should weigh in on the best way to approach this. I know in the past the preference is to leave concurrency compliance up to the developer. However, in the case of the default method -MR_countOfEntitiesWithPredicate:, if MR doesn't handle concurrency then assumptions have to be made as to which context to supply.
If MR is going to rely on the developer to comply with concurrency, then I personally lean towards removing the methods that do not explicitly ask for a context so there is no confusion as to what context and queue is being used.
Did my suggestion from the previous comment work for your situation?

Yes, it did. I've found a lot of similar issues as well, and I'm testing different patterns for making generic solutions. So far it's looking good, and it seems that I've found several of the issues that triggered CoreData could not fulfill a fault. I'll have to wait and see after my next deployment to be 100% sure though, most of my recorded crashes appear in production (just because of the amount of users).

I'm thinking that when I've got a better overview after working with it a bit, I want to include some suggestions into https://github.com/magicalpanda/MagicalRecord/blob/develop/Docs/Threads.md, both for how to avoid issues in the first place, and how to find them when they occur.

Thanks again for your help :)

You're welcome!

@Alydyn sorry for the lack of replies of late — I'm snowed under with other work.

You are correct in that I think concurrency compliance lies with the developer — however you're also correct in that the methods that don't accept a context make this difficult. Not sure what the best answer is yet — if I were starting from scratch, I'd leave out the methods that don't accept a context…

@thomassnielsen any contributions you'd like to make to the threading documentation would be appreciated (actually any help at all!). The best way to debug concurrency issues is to use Apple's concurrency debug flag.

@tonyarnold no worries. Between kids, baby and my own work I don't have many spare cycles either. I know you are still waiting on some things from me and I'm sorry about that. I will get to it, I promise!
Tony, are you the sole remaining active maintainer of MR now? I haven't seen much activity from others in quite a while.

@Alydyn it's still @casademora's baby, but I believe he's been busy with a new job as well. I'd really like to get 2.3.0 finished and then sit back and have a good think about what happens next (when I have time, of course!).

@tonyarnold understood! I will put a priority on those tests.

Just to inform that there is some progress here: I've done a bunch of debugging and fixing, found a couple of more bugs in my code, and I'm taking notes while I do it. I pushed out a new build of our app yesterday, hopefully to confirm that the last bugs are gone, or at least get some improved crash reports (added some scoping autorelease pools for thread debug sanity, thanks to a tip from the apple dev forums).

I'm thinking that I'll send a PR to the docs where I detail debugging with -com.apple.CoreData.ConcurrencyDebug 1, as well as when to create new contexts, and how to use them (context performBlock:, MR_inContext, etc) as soon as I've verified that my patterns work.

@thomassnielsen that would be greatly appreciated! Thanks for taking notes :)