nothirst / TICoreDataSync

Automatic synchronization for Core Data apps, between any combination of Mac OS X and iOS: Mac to iPhone to iPad to iPod touch and back again.
https://github.com/nothirst/TICoreDataSync/wiki
807 stars 61 forks source link

Why does [TICDSSynchronizationOperation processSyncChange:] need to refresh the object from the MOC? #62

Open ghost opened 11 years ago

ghost commented 11 years ago

I'm getting some SIGSEGV errors from my users' crash reports on line 450 of TICDSSynchronizationOperation.h,

- (void)processSyncChange:(TICDSSyncChange*)eachChange {
// ...
[eachChange.managedObjectContext refreshObject:eachChange mergeChanges:NO];
// ...
}

Is there a reason why the TICDSSyncChange's MOC needs to refresh the object? I presume that any changes made to it would be done on the same thread and thus wouldn't need to be refreshed. I am trying to find a reason for this SIGSEGV and can't seem to duplicate it and am thinking about just commenting out this line, I'm not sure exactly what it does or why it would cause a crash.

MrRooni commented 11 years ago

In that case we're using the refreshObject:mergeChanges: method to turn the TICDSSyncChange object back into a fault and reduce the sync's memory footprint. From the Apple documentation:

Turning object into a fault (flag is NO) means that strong references to related managed objects (that is, those to which object has a reference) are broken, so you can also use this method to trim a portion of your object graph you want to constrain memory usage.

Now of course that doesn't mean there isn't a bug here. I'm wondering if we're not doing some unintentional cross-thread access of the managed object context. Can you verify that sync change's managed object context is being accessed on the same thread where it was created?

ghost commented 11 years ago

There might be some cross-thread access. However the managed object context is only associated with the eachChange object (meaning the change set SQLite file), correct? Am I right in thinking that this does not refer to the main managed object context which accesses and changes the primary data store for the app?

I'm pretty sure that the sync change's managed object context is accessed on the same thread. I believe that TICDS is single-threaded, right? I know that it uses iOS 5's primary and secondary managed object context pattern, but for the most part the work that's done is all on the same thread as far as I can tell.

Honestly I have not yet been able to duplicate this crasher. I just see a lot of crashes coming in via my crash reporting tool and want to fix it.

MrRooni commented 11 years ago

However the managed object context is only associated with the eachChange object (meaning the change set SQLite file), correct?

Yes, that's correct.

Am I right in thinking that this does not refer to the main managed object context which accesses and changes the primary data store for the app?

Also correct.

I'm pretty sure that the sync change's managed object context is accessed on the same thread. I believe that TICDS is single-threaded, right?

It is single threaded within the confines of the synchronization operation, but that operation does run on its own thread. However, the managed object context being used by that operation should be getting created and used all from the same thread.

We have also seen similar crashes come in via our crash reporter and it is something that I'd like to get fixed, but like you I haven't been able to reproduce it.