Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

RKInMemoryEntityCache (maybe) and 'NSObjectInaccessibleException' 'CoreData could not fulfill a fault for' #611

Closed
alessandroorru opened this Issue · 4 comments

2 participants

Alessandro Orrù Blake Watters
Alessandro Orrù

Hello,

I found a bug in RKInMemoryEntityCache, I think. Here you can find a simple project, written ad-hoc to let the bug occur: http://d.pr/FOY2

Inside the zip you can find also two jsons, named news1.json and news2.json. Please copy them into a local web server and update the url in the app delegate.

You also need to link the project to the development branch of RestKit.

There are three steps involved. Please note that the steps are in a loop, and sometimes there is the need to let the app do more than 1 complete loop, in order to see the crash occur.

Step 1)
We fetch objects from news1.json, where we have 3 simple entities:

[
{
"identifier": "NEWS_001",
"name": "News #1",
"deleted": 0
},
{
"identifier": "NEWS_002",
"name": "News #2",
"deleted": 0
},
{
"identifier": "NEWS_003",
"name": "News #3",
"deleted": 0
}
]

Step 2)
We wait for a few seconds (needed for the crash to occur almost every time!), then we perform the same fetch request from ws.

Step 3)
We fetch objects from news2.json, where we have the same entities, but one with deleted=1 property.
After this fetch, we perform an NSFetchRequest to get all News objects with deleted=1 property, we delete them and we save the context.

[
{
"identifier": "NEWS_001",
"name": "News #1",
"deleted": 0
},
{
"identifier": "NEWS_002",
"name": "News #2",
"deleted": 0
},
{
"identifier": "NEWS_003",
"name": "News #3",
"deleted": 1
}
]

Step 1, again)
At this point in sqlite db there are just two items, NEWS_001 and NEWS_002.
So, when we fetch the news1.json (with all items with deleted=0), RestKit should create again NEWS_003, because for CoreData is a new item.

But here, app (usually) crashes, because the method below in InMemoryEntityCache returns the OLD NEWS_003 instance, that is not present in CoreData anymore. RestKit at this point tries to update that entity instead of creating a new one and there is the problem.

  • (NSManagedObject *)cachedObjectForEntity:(NSEntityDescription *)entity withMapping:(RKManagedObjectMapping *)mapping andPrimaryKeyValue:(id)primaryKeyValue inContext:(NSManagedObjectContext *)managedObjectContext
Blake Watters blakewatters was assigned
Blake Watters

Thank you for taking the time to pull this project together -- its very helpful. I have recreated the crash and I am looking into a fix. Will update shortly.

Blake Watters

@alessandroorru thank you so much for providing the example app highlighting the crash. It made it MUCH easier on me to debug. Please pull the sources from release/0.10.0 and verify the fix.

Alessandro Orrù

Hi Blake,

thanks for this commit.

Anyway, this fixes the crash problem, but it causes another big problem: if you let my test project run for a while (now without crashes) and then you open up the sqlite db with a viewer, you will see that EVERY SINGLE ENTITY has been duplicated!

I also tried using method existingObjectWithId:error: instead of your objectRegisteredForId:.
This should be a better method, because while objectRegisteredForId just checks if that objectId exists in cache's MOC, existingObjectWithId also check if an object with that objectId exists in persistent store, if it couldn't find it in MOC.

With this one, the duplication occurs only on the deleted items (in my test project, you will see just one NEWS_001 and NEWS_002, but a bunch of NEWS_003 items).

I digged a little in those problems, and I found a design issue.

The problem is that you keep track in memory in RKInMemoryEntityCache for an association between CoreData objectId and our primaryKey, and then you think "if an object with that objectId doesn't exist, then doesn't exist any object with that primaryKey". And, moreover, this cache doesn't get updated when an object is deleted.
So what happens? Happens that when NEWS_003 gets deleted (assume objectId is "3"), and then recreated, cache isn't updated so for you NEWS_003 is still objectId 3 in CoreData. So, the next time an update with ws occurs, containing NEWS_003, you still look for objectId 3, don't find it and assume it doesn't exist, and create it again and again.

A rapid solution would be to do a search in RKManagedObjectMapping looking directly to our primaryKey, if object fetched from cache is nil, before falling back into creating a new instance. Look at those snippets, I added some comments.

I hope this help into fixing this issue

RKInMemoryEntityCache
- (NSManagedObject *)cachedObjectForEntity:(NSEntityDescription *)entity
                               withMapping:(RKManagedObjectMapping *)mapping
                        andPrimaryKeyValue:(id)primaryKeyValue
                                 inContext:(NSManagedObjectContext *)managedObjectContext  {

    //----- CUT ------
    NSMutableDictionary *cachedObjectsForEntity = [self cachedObjectsForEntity:entity
                                                                   withMapping:mapping
                                                                     inContext:managedObjectContext];
    
    // NOTE: We coerce the primary key into a string (if possible) for convenience. Generally
    // primary keys are expressed either as a number of a string, so this lets us support either case interchangeably
    id lookupValue = [primaryKeyValue respondsToSelector:@selector(stringValue)] ? [primaryKeyValue stringValue] : primaryKeyValue;
    NSManagedObjectID *objectID = [cachedObjectsForEntity objectForKey:lookupValue];
    NSManagedObject *object = nil;
    if (objectID) {
        object = [self objectWithID:objectID inContext:managedObjectContext];
    }
    return object;
}
// RKManagedObjectMapping
- (id)mappableObjectForData:(id)mappableData {    

    //----- CUT ------

    // If we have found the primary key attribute & value, try to find an existing instance to update
    if (primaryKeyAttribute && primaryKeyValue) {
        object = [self.objectStore.cacheStrategy findInstanceOfEntity:entity
                                                     withMapping:self
                                              andPrimaryKeyValue:primaryKeyValue
                                          inManagedObjectContext:[self.objectStore managedObjectContextForCurrentThread]];
    }
   // HERE YOU SHOULD FIRST LOOK FOR AN OBJECT WITH OUR PRIMARY KEY
   // This code is not tested, it is just an example...
   if (object == nil) { 
      object = [entity objectWithPredicate:[NSPredicate predicateWithFormat:@"%@ == %@", primaryKeyAttribute, primaryKeyValue]];

     // here you should also update cache!
    }

    // IF YOU STILL CAN'T FIND OBJECT, THEN YOU SHOULD CREATE IT
    if (object == nil) {
        object = [[[NSManagedObject alloc] initWithEntity:entity
                           insertIntoManagedObjectContext:[_objectStore managedObjectContextForCurrentThread]] autorelease];
    }
    return object;
}
Blake Watters

Thank you for the thorough analysis -- I recreated similar research last night. There is obviously a large test coverage gap in place around duplicated objects that opened us up to this mess when the cacheStrategy refactoring was implemented by @jeffarena. I have just committed a change to the 0.10.0 release branch and the development branch that switches off the in memory cache store temporarily. I have a plan mapped out for how to refactor the in memory cache to be easier to grok and a bunch of tests to put down that will cover this situation going forward, but its probably going to be tomorrow or Monday before I can get it all pushed. Again, I really appreciate the help and I'll update this issue as I get the dev knocked out.

Bob Spryn sprynmr referenced this issue from a commit in sprynmr/RestKit
Blake Watters blakewatters Integrated primaryKey extension to NSEntityDescription and refactored…
… cache strategy

classes to eliminate issues with duplicated objects. closes #611, #612, #613, #618

* NSEntityDescription is now aware of the primaryKeyAttribute. Can be configured via
Interface Builder within Xcode or programatically.
* Added findByPrimaryKey: interface to the Core Data extensions.
* Relaxed dependencies on RKManagedObjectMapping across the system now that primaryKey is
available without a reference to the mapping.
a545c39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.