3

I'm trying to separate my application work when there is a bigger work to do to optimize performance. My problem is about a NSManagedObjectContext used in another thread than the main one.

I'm calling:

[NSThread detachNewThreadSelector:@selector(test:) toTarget:self withObject:myObject];

On the test method there are some stuff to do and I have a problem here:

NSArray *fetchResults = [moc
                         executeFetchRequest:request
                         error:&error];

Here is my test method:

-(void) test:(MyObject *)myObject{
  @autoreleasepool {
    //Mycode
  }
}

The second time I call the test method, my new thread is blocked when the executeFetchRequest is called. This problem arrived when my test method is called more than one time in succession. I think the problem comes from the moc but I can't really understand why.

Edit:

With @Charlie's method it's almost working. Here is my code to save my NSManagedObjectContext (object created on my new thread).

- (void) saveContext:(NSManagedObjectContext *) moc{
  NSError *error = nil;
  if ([moc hasChanges] && ![moc save:&error]) {
    NSLog(@"Unresolved error %@, %@", error, [error userInfo]);
  }
}

This method is called on the new thread. My problem now is that with this save, I have a deadlock and I don't really understand why. Without it's perfectly working.

Edit2

I'm working on this issue but I still can't fix it. I changed my code about the detachNewThreadSelector. Here is my new code:

NSManagedObjectContext* context = [[NSManagedObjectContext alloc]
                                   initWithConcurrencyType:NSPrivateQueueConcurrencyType];
context.persistentStoreCoordinator = self.persistentStoreCoordinator;
context.undoManager = nil;

[context performBlock:^
 {
     CCImages* cachedImage;
     NSManagedObjectContext *childContext = [[NSManagedObjectContext alloc] initWithConcurrencyType:NSPrivateQueueConcurrencyType];
     childContext.parentContext = context;
     cachedImage=[CCImages getCCImageForKey:path inManagedObjectContext:childContext];

     UIImage *image = [self getImageFromCacheWithPath:path andCachedImage:cachedImage atDate:now];
    if (image != nil){
         if(![weakSelf.delegate respondsToSelector:@selector(CacheCacheDidLoadImageFromCache:)])
             [weakSelf setDelegate:appDelegate.callbacksCollector];
         //[weakSelf useCallbackCollectorForDelegate:weakSelf inMethod:@"initPaginatorForListMoments"];
         [weakSelf.delegate CacheCacheDidLoadImageFromCache:image];
     }
}

- (UIImage*) getImageFromCacheWithPath:(NSString*) path andCachedImage:(CCImages *) cachedImage atDate: (NSDate *) now{

  NSURL* localURL=[NSURL URLWithString:cachedImage.path relativeToURL:[self imageCacheDirectory]];

  UIImage * image;
  //restore uiimage from local file system
  if (localURL) {
    image=[UIImage imageWithContentsOfFile:[localURL path]];

    //update cache
    [cachedImage setLastAccessedAt:now];
    [self saveContext];

    if(image)
        return image;
  }
  return nil;

}

Just after that, I'm saving my contexts (manually for now)

[childContext performBlock:^{
         NSError *error = nil;
         if (![childContext save:&error]) {
             DDLogError(@"Error during context saving when getting image from cache : %@",[error description]);
         }
         else{
             [context performBlock:^{
                 NSError *error = nil;
                 if (![context save:&error]) {
                     DDLogError(@"Error during context saving when getting image from cache : %@",[error description]);
                 }
             }];
         }
     }];

There is a strange problem. My call back method is called without any problem on my controller (which implements the CacheCacheDidLoadImageFromCache: method). On this method I attest the reception of the image (DDLogInfo) and say that I want my spinner to stop. It does not directly but only 15secondes after the callback method was called.

My main problem is that my context (I guess) is still loading my image from the cache while it was already found. I said 'already' because the callback method has been called and the image was present. There is no suspicious activity of the CPU or of the memory. Instruments didn't find any leak.

I'm pretty sure that I'm using wrongly the NSManagedObjectContext but I can't find where.

brcebn
  • 1,571
  • 1
  • 23
  • 46
  • 1
    You can't do this. You should use queue-contained concurrency types on managed object contexts now. Somebody may post a snippet of useful code here, but you should google concurrency and core data and make sure you see them talking about `performBlock:` and `performBlockAndWait:`. – Jason Coco Jun 17 '14 at 06:20
  • Thank you for your fast answer ! I'm not sure to understand what you mean. What can't I do ? At the beginning I was stating the `test` method without starting a new thread and it was working. I guess that something went wrong between my `moc` and my new thread. You said 'queue-contained concurrency types on managed object contexts'. I have no idea how to do it. Do you think that my problem comes from `detachNewThreadSelector` or directly from `executeFetchRequest`? – brcebn Jun 17 '14 at 06:27
  • 1
    You can't actually use the managed object context or *any* of the managed objects you get from it on another thread. It's not legal and will result in crashes, deadlocks, data corruption, or a whole host of undefined behavior. There are many tutorials available online that discuss good core data concurrency patterns. – Jason Coco Jun 17 '14 at 06:37
  • 1
    Jason Coco is absolutely correct. You should NOT attempt your current approach, instead use queue confinement and `performBlock:`. You are currently using the thread confinement model, which is deprecated, and are not using it correctly, which will result in crashes, deadlocks, etc. – quellish Jun 17 '14 at 20:04

2 Answers2

9

You are using the old concurrency model of thread confinement, and violating it's rules (as described in the Core Data Concurrency Guide, which has not been updated yet for queue confinement). Specifically, you are trying to use an NSManagedObjectContext or NSManagedObject between multiple threads. This is bad. Thread confinement should not be used for new code, only to maintain the compatibility of old code while it's being migrated to queue confinement. This does not seem to apply to you.

To use queue confinement to solve your problem, first you should create a context attached to your persistent store coordinator. This will serve as the parent for all other contexts:

+ (NSManagedObjectContent *) parentContextWithPersistentStoreCoordinator:(NSPersistentStoreCoordinator *)coordinator {
    NSManagedObjectContext  *result = nil;

    result = [[NSManagedObjectContext alloc] initWithConcurrencyType:NSPrivateQueueConcurrencyType];
    [result setPersistentStoreCoordinator:coordinator];

    return result;
}

Next, you want the ability to create child managed object contexts. You will use these to perform work on the data, wether reading or writing. An NSManagedObjectContext is a scratchpad of the work you are doing. You can think of it as a transaction. For example, if you're updating the store from a detail view controller you would create a new child context. Or if you were performing a multi-step import of a large data set, you would create a child for each step.

This will create a new child context from a parent:

+ (NSManagedObjectContext *) childContextWithParent:(NSManagedObjectContext *)parent {
    NSManagedObjectContext  *result = nil;

    result = [[NSManagedObjectContext alloc] initWithConcurrencyType:NSPrivateQueueConcurrencyType];
    [result setParent:parent];

    return result;
}

Now you have a parent context, and you can create child contexts to perform work. To perform work on a context, you must wrap that work in performBlock: to execute it on the context's queue. I do not recommend using performBlockAndWait:. That is intended only for re-rentrant methods, and does not provide an autorelease pool or processing of user events (user events are what drives nearly all of Core Data, so they're important. performBlockAndWait: is an easy way to introduce bugs).

Instead of performBlockAndWait: for your example above, create a method that takes a block to process the results of your fetch. The fetch, and the block, will run from the context's queue - the threading is done for you by Core Data:

- (void) doThingWithFetchResults:(void (^)(NSArray *results, NSError *error))resultsHandler{
    if (resultsHandler != nil){
        [[self context] performBlock:^{
            NSArray *fetchResults = [[self context] executeFetchRequest:request error:&error];
            resultsHandler(fetchResults, error);
        }];
    }
}

Which you would call like this:

[self doThingsWithFetchResults:^(NSArray *something, NSError *error){
    if ([something count] > 0){
      // Do stuff with your array of managed objects
    } else {
      // Handle the error
    }
}];

That said, always prefer using an NSFetchedResultsController over using executeFetch:. There seems to be a belief that NSFetchedResultsController is for powering table views or that it can only be used from the main thread or queue. This is not true. A fetched results controller can be used with a private queue context as shown above, it does not require a main queue context. The delegate callbacks the fetched results controller emits will come from whatever queue it's context is using, so UIKit calls need to be made on the main queue inside your delegate method implementations. The one issue with using a fetched results controller this way is that caching does not work due to a bug. Again, always prefer the higher level NSFetchedResultsController to executeFetch:.

When you save a context using queue confinement you are only saving that context, and the save will push the changes in that context to it's parent. To save to the store you must recursively save all the way. This is easy to do. Save the current context, then call save on the parent as well. Doing this recursively will save all the way to the store - the context that has no parent context.

Example:

- (void) saveContextAllTheWayBaby:(NSManagedObjectContext *)context {
[context performBlock:^{
        NSError *error  = nil;
        if (![context save:&error]){
            // Handle the error appropriately.
        } else {
            [self saveContextAllTheWayBaby:[context parentContext]];
        }

    }];

}

You do not, and should not, use merge notifications and mergeChangesFromContextDidSaveNotification: with queue confinement. mergeChangesFromContextDidSaveNotification: is a mechanism for the thread confinement model that is replaced by the parent-child context model. Using it can cause a whole slew of problems.

Following the examples above you should be able to abandon thread confinement and all of the issues that come with it. The problems you are seeing with your current implementation are only the tip of the iceberg.

There are a number of Core Data sessions from the past several years of WWDC that may also be of help. The 2012 WWDC Session "Core Data Best Practices" should be of particular interest.

quellish
  • 21,123
  • 4
  • 76
  • 83
  • Such a excellent tutorial ! I may understand much better now. I still have a couple of question. On the `doThingWithFetchResults` method. I'm a bit lost about which context is parent and which is the child. (there are `context` and `moc` variable). Could you add more details (even if it's very well detailed all around your answer). My second question is about the saving. How is it possible that it's not an endless circle ? Should an error appear to stop it ? – brcebn Jun 18 '14 at 13:21
  • One more question: Is `performBlock` actually directly creating a new thread ? – brcebn Jun 18 '14 at 16:33
  • performBlock executes the block on the context's queue. It's up to the system to decide wether to use an existing, idle thread or to create a new one. – quellish Jun 18 '14 at 17:31
  • It won't be an endless circle because the bottom most context will not have a parent. It will have a persistent store coordinator instead. – quellish Jun 18 '14 at 17:32
  • Thank you for your other answers. By the way. I'm currently not able to implement what you said. I've added my problem evolution to my main question. – brcebn Jun 19 '14 at 18:29
  • It's not clear from your update that your current problem is a Core Data problem. Please elaborate on this: "My main problem is that my is still loading my image from the cache while it was already found (call back method)." What is loading the image? How? It is just using the UIImage you passed to the delegate? It looks like you are getting cachedImage from Core Data, but then passing it to another method that returns and image which is then handed off to your delegate? – quellish Jun 19 '14 at 20:45
  • I agree. I'll try to explain better. My implementation of my callback method is supposed to stop the spinner. This method is called but the spinner is not stop directly. Something happens (I don't know what) when I'm calling the `UIImage` from Core Data. Finally the spinner is stopped 15sec after the callback method was called. I've added some code and explanations on the main subject. – brcebn Jun 19 '14 at 21:06
  • In the above code you are invoking your delegate callback from inside the context's performBlock:, which means it's running on a thread managed by the context's queue. That delegate method is accesses UIKit to stop the spinner, which *must* be done from the main thread/queue. That explains the behavior you are seeing, and is likely to lead to other problems as well. Inside your delegate method, or where it's being called, have it do that on the main queue. – quellish Jun 19 '14 at 22:12
  • 1
    Like so: `[[NSOperationQueue mainQueue] addOperationWithBlock:^{ // UIKit stuff goes here }];` – quellish Jun 19 '14 at 22:13
  • Wonderful ! Thank you very much ! I think I'm still far away to understand all this mechanism. Thank you again ! – brcebn Jun 19 '14 at 22:46
  • Godlike answer / response. Thank you so much. – yujean Sep 11 '15 at 20:18
0

if you want to use managed object context in background thread, there are two approaches,

1 Create a new context set concurrency type to NSPrivateQueueConcurrencyType and set the parentContext to main thread context

2 Create a new context set concurrency type to NSPrivateQueueConcurrencyType and set persistentStoreCoordinator to main thread persistentStoreCoordinator

dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^(void) {

    NSManagedObjectContext *privateContext = [[NSManagedObjectContext alloc] initWithConcurrencyType:NSPrivateQueueConcurrencyType];
    privateContext.persistentStoreCoordinator = mainManagedObjectContext.persistentStoreCoordinator;

    [[NSNotificationCenter defaultCenter] addObserverForName:NSManagedObjectContextDidSaveNotification object:nil queue:nil usingBlock:^(NSNotification* note) {
        NSManagedObjectContext *moc = mainManagedObjectContext;
        if (note.object != moc) {
            [moc mergeChangesFromContextDidSaveNotification:note];
        }
    }];

    // do work here
    // remember managed object is not thread save, so you need to reload the object in private context
});

before exist the thread, make sure remove the observer, bad thing can happen if you don't

for more details read http://www.objc.io/issue-2/common-background-practices.html

Charlie Wu
  • 7,657
  • 5
  • 33
  • 40
  • I'm trying to do what you said but I have the same problem. I'm sure I'm doing it wrong but I don't really understand what I'm doing. On my cache class (where the `moc` is called) I have a persistentStoreCoordinator like described [here](http://stackoverflow.com/a/1897580/2616474). I've edited my main question by adding my try. – brcebn Jun 17 '14 at 07:43
  • you should create moc in the tread you will use the managed object, so in thread create a new private context, when private context changes, call save:, the notification center that observing NSManagedObjectContextDidSaveNotification will run the code that merge private context with the main context, again read the article i posted – Charlie Wu Jun 17 '14 at 07:50
  • OK ! Now it's clear. I've read this document but it was not as clear as what you've just said. :-) – brcebn Jun 17 '14 at 09:51
  • Do I really need to save the context if my request is just a research ? If I do, is it only to have the notification (and merge the new moc with the main one) ? – brcebn Jun 17 '14 at 10:23
  • it's working without calling save: With I have a deadlock. Do you have an idea ? I've updated my main post with some code. – brcebn Jun 17 '14 at 19:32
  • You should NOT use merge notifications with parent child contexts. That misses the whole point of parent child contexts - the chain of contexts handles the changes. Children push changes to parents, which then push to the store. – quellish Jun 17 '14 at 20:07
  • if you only reading off the context, you don't need to save or merge and you can stop reading. If you are writing, you need to save context. If you use parent context, you can just save ([moc save:]), but personally I had a lot of issue using parent context. If you set persistentStoreCoordinator, you to save [moc save:] and the notification and merge block. – Charlie Wu Jun 19 '14 at 02:03