Reset all managed object contexts in all threads #110

Closed
idpaterson opened this Issue Dec 13, 2011 · 10 comments

Comments

Projects
None yet
6 participants
Contributor

idpaterson commented Dec 13, 2011

Our app requires a full reset capability to allow a tester to reset the app, without closing it, to essentially the same state it would be in after an install. Part of that process is rather harsh - we delete the main CoreData sqlite store.

Unfortunately, on some resets a thread is used such that the existing cached context for that thread is no longer valid. This allows old data to be returned from memory which can have a multitude of consequences. I would like to propose +[NSManagedObjectContext resetContextForAllThreads]

That method could be implemented by incrementing a static version number and also storing the current version number in the threadDictionary. Any call to + [NSManagedObjectContext contextForCurrentThread] would validate against the current version number and generate a new context if it is outdated.

Is this something that would be useful for anyone else, or worth adding to the project? We will be implementing the solution described above since it is a fairly simple workaround, but if anyone else has a better idea or knows of a way to avoid this problem, we're all ears.

@ghost

ghost commented Dec 13, 2011

interesting proposal. A couple of thoughts off the top of my head:

  1. Please use MR_ as a prefix to all your category methods. I haven't documented everything just yet, but in order to avoid future collisions with system (apple's) libraries, the prefix is necessary.

  2. I think this would work well in the cleanup method. The reason to have a cleanup method is to reset the state of the entire stack. I think since the cleanup method already resets the default context, it's not unreasonable to expect it to also reset any contexts that are wandering around anyway.

  3. Would it be impractical to have the reset trigger be a notification? Notifications span all threads (as is painfully obvious with UI updating notifications from a background thread). You can just "notify" all background contexts that it's time to reset, and go away...just a thought.

Otherwise, I look forward to seeing this implemented.

Contributor

idpaterson commented Dec 13, 2011

Thanks for the quick response. It seems that our copy of MagicalRecord is outdated, I see the new naming scheme now. I am unsure about the implications of 3), but that would likely have less overall impact for those who do not need the feature. I certainly agree that this would be appropriate to trigger in the cleanup method.

This has been successfully implemented in our project as originally described, though we had to create a brand new context; calling reset on the existing context still resulted in old data to be returned. That does not seem like it should happen, but resetting did not work for us. We are using ARC, though it is disabled for MR and other libraries. The particular managed object we are loading has only zero or one instances and it is therefore always loaded with [TheClassName findFirst]; - again, old MR code so this might behave differently after an update. I'll try to take some time to revisit this after updating to the latest MR.

@ghost

ghost commented Dec 13, 2011

Other than the naming scheme, I truly hope behavior has not changed too much with the more recent updates. While I'm not trying to create a large legacy codebase with ultimate backwards compatibility, I do want to make sure it works the same on the current SDK as well as the last couple of releases. If you do find a large discrepancy, please file a bug with the differences so they can be corrected.

Thanks!

Saul Mora
Founding Panda
saul@magicalpanda.com

On Dec 13, 2011, at 9:45 AM, Ian Paterson wrote:

Thanks for the quick response. It seems that our copy of MagicalRecord is outdated, I see the new naming scheme now. I am unsure about the implications of #3, but that would likely have less overall impact for those who do not need the feature. I certainly agree that this would be appropriate to trigger in the cleanup method.

This has been successfully implemented in our project as originally described, though we had to create a brand new context; calling reset on the existing context still resulted in old data to be returned. That does not seem like it should happen, but resetting did not work for us. We are using ARC, though it is disabled for MR and other libraries. The particular managed object we are loading has only zero or one instances and it is therefore always loaded with [TheClassName findFirst]; - again, old MR code so this might behave differently after an update. I'll try to take some time to revisit this after updating to the latest MR.


Reply to this email directly or view it on GitHub:
#110 (comment)

We have exact the same problem for our UnitTests. We build up a new stack in setUp of each test, filling the store with test data. After tearDown these contextForCurrentThread keep their reference to the old persistent store.

Is there an implementation of a clean up method that releases the context for all threads?

Contributor

idpaterson commented Mar 21, 2012

aarsland, do you have better results if you modify NSManagedObjectContext+MagicalRecord.m to remove the context completely? Each context is stored in its NSThread's userInfo which in your case means that the next time you happen to work with a thread previously used, the old persistent store will be used. As mentioned earlier, simply calling -[NSManagedObjectContext reset] did not release the references to everything that had changed in our batched import process. This approach, even in our case where the persistent store does not change, has been working very well.

+ (void)MR_resetContextForCurrentThread
{
    [[NSManagedObjectContext MR_contextForCurrentThread] reset];

    NSMutableDictionary *threadDict = [[NSThread currentThread] threadDictionary];
    [threadDict removeObjectForKey:kMagicalRecordManagedObjectContextKey];
}

In order to reset the context for all threads, we added an NSNumber to each threadDictionary signifying its "version." In order to reset all contexts on all threads, we bump a static version number and compare the two in -[NSManagedObjectContext MR_contextForCurrentThread] to determine whether the context is still valid. Unlike the code posted above this does not release memory form the old contexts immediately, but instead once they are used again.

aarsland commented Apr 4, 2012

Since we just needed to fix this for our unit test, I made a static NSDictionary with contexts for all threads. Then I added a reset method which empties this dictionary at each tearDown to make sure setUp gets a clean starting point. Thanks for your input.

Contributor

tonyarnold commented Jun 26, 2012

I'm not sure I'm understanding the function of the "reset" method — from my reading of the documentation, it should empty the entire MOC, is that right? If so, this is not working for me on an ARC-enabled project I'm working on right now for iOS.

Edit: My Bad — I actually wanted to delete all objects, so this works just fine:

https://gist.github.com/2995892

blackgold9 closed this Oct 9, 2012

aviell commented Dec 1, 2012

@idpaterson you mentioned that calling [NSManagedObjectContext reset] isn't working , I saw this behavior as well for my tests.
Do you have any idea why?

@magicalpanda @blackgold9 If resetting the context is not doing the job, why not add @idpaterson suggestion to also remove the context from the thread dictionary to the code base? Am I missing something?

Thanks.

Contributor

idpaterson commented Dec 1, 2012

@aviell, unfortunately I never attempted to create an isolated test to confirm that observation, so I can't quite say whether it was something specific to Core Data, MagicalRecord, or a particular aspect of my code. In my case, removing the reference to the NSManagedObjectContext in the threadDictionary was the only approach that worked. We were using thread confinement concurrency rather than the newer private queue concurrency with parent contexts.

We have since reduced our reliance on Core Data to avoid the massive imports that originally caused problems, so I am not able to verify the above approach anymore.

aviell commented Apr 4, 2013

Hey I just noticed your answer. thanks @idpaterson

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment