MR_contextForCurrentThread used of ThreadContext is problematic

[carlshimer]

General problem:

Our application supports the ability for the user to log out and log in again with a different user name / password. We were seeing strange issues and /or crashes with core Data after a user logged out and logged in again.

Here is what we are doing under the hood:

1. When user clicks logout, we shutdown all our operation queues, clean up Magical record using the cleanup method, and finally delete the sqlite database.
2. User logs in, we set up core data / magical record from scratch.
3. Download data from the server.

What I noticed is that was that on some occasions it looked like we were getting data from the old database despite the fact that it had been deleted and Magical record had been cleaned up / shutdown. I finally tracked down the problem to MR_contextForCurrentThread returning a NSManagedObjectContext from the ThreadContext that was created with the old database. Said another way, despite shutting down my operation queues as cleanly as possible the system is still reusing threads and the thread local storage associated with them.

I figured out that what I really need was to iterate through all of my threads and call MR_resetContextForCurrentThread. I honestly don't know of an easy way to do that - you can iterate threads at the C level using mach based APIs but not sure you can get access to the ThreadContext. Another possibility is to broadcast a message to all threads using notifications but this seems problematic as well since you don't have control over which thread runs the notification handler.

The fundamental problem seems to be that OperationQueue's use threads but want to hide the details from you while CoreData absolutely requires you to be aware of thread management.

My current solution is to use a global runcount unsigned integer that I increment when the user logs out. I modified MR_contextForCurrentThread to check for this global variable (via the AppDelegate) and reset the thread local MOC. So far this has fixed my problems.

Has anyone run into this problem or can offer an alternative solution? I can make this into a pull request if anyone is interested...

[casademora]

This is the exact reason I plan on deprecating this method. With queues
basically reusing threads, this scenario is far more common than people
think.

My advice, use explicit contexts for all your queues. That is, you need to
keep track of the context yourselves for now while we figure out a nice way
to make this work with queues.

Saul Mora
@casademora

On Sep 7, 2013, at 5:45 PM, Carl Shimer notifications@github.com wrote:

General problem:

Our application supports the ability for the user to log out and log in
again with a different user name / password. We were seeing strange issues
and /or crashes with core Data after a user logged out and logged in again.

Here is what we are doing under the hood:

1. When user clicks logout, we shutdown all our operation queues, clean up
   Magical record using the cleanup method, and finally delete the sqlite
   database.
2. User logs in, we set up core data / magical record from scratch.
3. Download data from the server.

What I noticed is that was that on some occasions it looked like we were
getting data from the old database despite the fact that it had been
deleted and Magical record had been cleaned up / shutdown. I finally
tracked down the problem to MR_contextForCurrentThread returning a
NSManagedObjectContext from the ThreadContext that was created with the old
database. Said another way, despite shutting down my operation queues as
cleanly as possible the system is still reusing threads and the thread
local storage associated with them.

I figured out that what I really need was to iterate through all of my
threads and call MR_resetContextForCurrentThread. I honestly don't know of
an easy way to do that - you can iterate threads at the C level using mach
based APIs but not sure you can get access to the ThreadContext. Another
possibility is to broadcast a message to all threads using notifications
but this seems problematic as well since you don't have control over which
thread runs the notification handler.

The fundamental problem seems to be that OperationQueue's use threads but
want to hide the details from you while CoreData absolutely requires you to
be aware of thread management.

My current solution is to use a global runcount unsigned integer that I
increment when the user logs out. I modified MR_contextForCurrentThread to
check for this global variable (via the AppDelegate) and reset the thread
local MOC. So far this has fixed my problems.

Has anyone run into this problem or can offer an alternative solution? I
can make this into a pull request if anyone is interested...

—
Reply to this email directly or view it on
GitHubhttps://github.com//issues/566
.

[carlshimer]

Sweet. Is it safe to use one MOC per queue even if you can have multiple threads per queue?

[casademora]

From what I can tell so far, on moc per queue works ok because your access
to that moc is fairly limited and doesn't cross threads.

Saul Mora
@casademora

On Sep 7, 2013, at 5:58 PM, Carl Shimer notifications@github.com wrote:

Sweet. Is it safe to use one MOC per queue even if you can have multiple
threads per queue?

—
Reply to this email directly or view it on
GitHubhttps://github.com//issues/566#issuecomment-24012236
.

[carlshimer]

I have tried implementing a solution that uses one Moc per OperationQueue. basically, I subclass NSOperationQueue and added a method that creates and caches an NSOperationQueue whenever someone calls queueContext.

// lookup a managed object in the current context for the operation queue
-(NSManagedObjectContext*)contextForCurrentQueue
{
    if(!queueContext) {
        queueContext = [NSManagedObjectContext MR_contextWithParent:[NSManagedObjectContext MR_defaultContext]];
        [queueContext MR_setWorkingName:[self name]];
    }
    return queueContext;
}

+(NSManagedObjectContext*)queueContext
{
    MBOperationQueue* pSelf = [self currentQueue];
    // you shouldn't be calling this if you are not on an operation queue and should be
    // a clear developer error.
    assert(pSelf);
    // verify that we are on our derived class and not something else!
    assert([pSelf isKindOfClass:[MyDerivedOperationQueueClass class]]);
    return [pSelf contextForCurrentQueue];
}

In turn, I modified MR_contextForCurrentThread to look something ike this:

if ([NSThread isMainThread])
    {
        return [self MR_defaultContext];
    }
    else {
        return [MyDerivedOperationQueueClass queueContext];
    }

Sadly, this approach sort of works but exhibits other strange crashing errors (more than before). Any ideas?

[casademora]

Im not sure this auto storage of a context based on anything is a good
idea. I could see perhaps creating an nsoperation subclass with a context
already as a helper, but it's not giving you much without your own
threading logic. I just don't think contextForCurrentThread or a context
for queue is a good solid pattern

Saul Mora
@casademora

On Sep 8, 2013, at 7:34 PM, Carl Shimer notifications@github.com wrote:

I have tried implementing a solution that uses one Moc per OperationQueue.
basically, I subclass NSOperationQueue and added a method that creates and
caches an NSOperationQueue whenever someone calls queueContext.

// lookup a managed object in the current context for the operation
queue-(NSManagedObjectContext_)contextForCurrentQueue{
if(!queueContext) {
queueContext = [NSManagedObjectContext
MR_contextWithParent:[NSManagedObjectContext MR_defaultContext]];
[queueContext MR_setWorkingName:[self name]];
}
return queueContext;}
+(NSManagedObjectContext_)queueContext{
MBOperationQueue* pSelf = [self currentQueue];
// you shouldn't be calling this if you are not on an operation
queue and should be
// a clear developer error.
assert(pSelf);
// verify that we are on our derived class and not something else!
assert([pSelf isKindOfClass:[MyDerivedOperationQueueClass class]]);
return [pSelf contextForCurrentQueue];}

In turn, I modified MR_contextForCurrentThread to look something ike this:

if ([NSThread isMainThread])
{
return [self MR_defaultContext];
}
else {
return [MyDerivedOperationQueueClass queueContext];
}

Sadly, this approach sort of works but exhibits other strange crashing
errors (more than before). Any ideas?

—
Reply to this email directly or view it on
GitHubhttps://github.com//issues/566#issuecomment-24034512
.

[daveanderson]

I ran into this where tearing down and creating a new dispatch_queue (target_queue) between unit test runs was reusing the same thread and thus the same MR_contextForCurrentThread cached in the thread dictionary.

In the -dealloc of the class where I'm creating my dispatch_queue I tried

- (void)dealloc {

    dispatch_async(target_queue, ^{
        [NSManagedObjectContext MR_resetContextForCurrentThread];
    });
}

to no avail.

I've since replaced this with

- (void)dealloc {

    dispatch_async(target_queue, ^{
        [NSManagedObjectContext MR_resetContextForCurrentThread];
        [NSManagedObjectContext MR_resetCurrentThreadDictionary];
    });
}

where I've added

+ (void) MR_resetCurrentThreadDictionary {
    NSMutableDictionary *threadDict = [[NSThread currentThread] threadDictionary];
    NSManagedObjectContext *threadContext = [threadDict objectForKey:kMagicalRecordManagedObjectContextKey];
    if (threadContext != nil)
    {
        [threadDict removeObjectForKey:kMagicalRecordManagedObjectContextKey];
    }
}

to explicitly remove the cached MOC.

For NSOperationQueue you could have an "cleanup" operation that explicitly removes the MOC from the current thread dictionary as necessary, but its still not a great solution because it's a manual kludge.

[daveanderson]

My attempt still isn't as good as just managing the contexts yourself. Had to change it.

[grittymindy]

I've read this issue description and all the comments, also the post by @casademora. Still don't know why a reused thread and the cached MOC in thread context will cause the crash. The MOC is not used by a different thread, but the same one, right? Can you explain that in detail, please? @carlshimer @casademora @daveanderson Thanks a lot!

[tonyarnold]

So the most basic answer is this: threads do not equate to GCD queues, so stashing against the current thread is no guarantee that the queue will end up using the same thread in future. Also, as of iOS 9/OS X 10.11, confinement contexts are deprecated.

Core Data uses queues to manage much of it's thread confinement in more recent versions, so stashing a private (or main) queue NSManagedObjectContext into a thread's dictionary is no guarantee that that thread is the one that the context will use (in fact, I can guarantee you that it won't be).

You should be using -[NSManagedObjectContext performBlock:…] and -[NSManagedObjectContext performBlockAndWait:…] in your apps now, and forget about managing threads yourself where Core Data is involved entirely.

Apple provides more detail on how to work with Core Data and concurrency in their Core Data Programming Guide. It's also probably worthwhile gaining a deeper understanding of dispatch queues too: Concurrency Programming Guide — Dispatch Queues

[grittymindy]

@tonyarnold Thanks for the detailed explanation. That is very clear.