Modifying a mutable object in a completion handler

Question

I have a thread safety question in the following code example from Apple (from the GameKit programming guide)

This is for downloading achievements from the game center and saving it locally:

Step 1) Add a mutable dictionary property to your class that reports achievements. This dictionary stores a collection of achievement objects.

@property(nonatomic, retain) NSMutableDictionary *achievementsDictionary; 

Step 2) Initialize the dictionary of achievements.

 achievementsDictionary = [[NSMutableDictionary alloc] init]; 

Step 3) Modify your code that loads load achievement data to add achievement objects to the dictionary.

 { [GKAchievement loadAchievementsWithCompletionHandler:^(NSArray *achievements, NSError *error) { if (error == nil) { for (GKAchievement* achievement in achievements) [achievementsDictionary setObject: achievement forKey: achievement.identifier]; } }]; 

My question is this: the successDictionary object changes in the completion handler without any sort locks. Is this allowed since completion handlers are a unit of work that will be guaranteed by iOS that will execute as a unit in the main thread? And never run into thread safety issues?

In another sample Apple code (GKTapper), this part is handled differently:

 @property (retain) NSMutableDictionary* earnedAchievementCache; // note this is atomic 

Then in the handler:

 [GKAchievement loadAchievementsWithCompletionHandler: ^(NSArray *scores, NSError *error) { if(error == NULL) { NSMutableDictionary* tempCache= [NSMutableDictionary dictionaryWithCapacity: [scores count]]; for (GKAchievement* score in scores) { [tempCache setObject: score forKey: score.identifier]; } self.earnedAchievementCache= tempCache; } }]; 

So, why is another style one of the ways more correct than the other?

Answer 1

Is this allowed because completion handlers are the unit of work that iOS will guarantee to execute as units in the main thread? And never run into thread safety issues?

This is definitely not the case. The documentation for -loadAchievementsWithCompletionHandler: explicitly warns that the completion handler may be called on a thread other than the one you started the download from.

Apple's "Thread Programming Guide" classifies NSMutableDictionary among unsafe classes, but qualifies it as follows: "In most cases, you can use these classes from any thread if you use them from only one thread at a time."

So, if both applications are designed in such a way that nothing will work with the achievement cache until the work task completes its update, then no synchronization is needed. This is the only way that the first example can be considered safe, and it represents little security.

The last example looks like it relies on support for atomic properties to make a switch between the old cache and the new cache. This should be safe if all access to the property is through his accessory, and not direct access to ivar. This is due to the fact that accessors are synchronized with each other, so you do not run the risk of seeing half the value. In addition, getter saves and implements the return value, so that the code with the old version will be able to finish working without fail, because it was released in the middle of its work. The non-atomic getter simply returns the object directly, which means that it can be freed from your code if a new value was set for this property by another thread. Direct ivar access may run into the same problem.

I would say that the last example is correct and elegant, although perhaps a little more subtle without commentary, explaining how important the atomicity of the property is.