I was watching this WWDC talk called Thread Sanitizer and Static Analysis where the speaker shows us there is a data race if two different threads call notifyStartNetworkActivity
:
var activityCount: Int = 0
public class ActivityCounter : NSObject {
public func notifyStartNetworkActivity() {
activityCount = activityCount + 1
self.updateNetworkActivityUI()
}
func updateNetworkActivityUl() {
WWDCJSONOperation.prepareNetworkActivity()
if activityCount > 0 {
WWDCJSONOperation.visibilityTimer?.invalidate()
WWDCJSONOperation.visibilityTimer = nil
UIApplication.shared().isNetworkActivityIndicatorVisible = true
} else {
/* To prevent the indicator from flickering on and off, we delay the hiding of the indicator by one second. This provides the chance to come in and invalidate the timer before it fires. */
WWDCJSONOperation.visibilityTimer = Timer.scheduledTimer(timelnterval: 1.0, target: self, selector: #selector(ActivityCounter.fire(timer:)))
...
At 6:45, the speaker then says:
Now, I could have fixed this race by adding a lock. But notice that this is just a symptom. The next line here updates the UI. And we know that the UI updates should happen on the main thread. So the proper fix here is to dispatch both the counter increment and the UI update onto the main queue with Grand Central Dispatch. This will both take care of the logical problem in our application and also take care of the race because all the threads will access that count variable from the same thread.
public func notifyStartNetworkActivity() {
DispatchQueue.main.async {
activityCount = activityCount + 1
self.updateNetworkActivityUI()
}
}
The problem I have with this fix is that we're adding unnecessary work to the main thread. Obviously the UIKit calls such as UIApplication.shared().isNetworkActivityIndicatorVisible = true
need to be done on the main thread as UIKit is not thread safe. But there is unnecessary work done on the main thread such as updating activityCount
. Unnecessary work is bad as explained in other WWDC talks I've watched, including Optimizing Your App for Multitasking on iPad in iOS 9:
So the most important thing you can do to keep your app responsive is to do as little work as possible on your main thread. The main thread's top priority is to respond to user events, and doing unnecessary work on your main thread means that the main thread has less time to respond to user events.
Therefore, in this case I would've used a lock or GCD queue to control access. While these add overhead, this overhead is added to background threads doing network operations, so we can keep the UI as responsive as possible. The speaker however is obviously far more knowledgeable on multithreading than I am, so I am curious as to why in this case the speaker says the proper fix involves adding non-UIKit work to the main thread.
The amount of work to increment and update this model property is inconsequential and given that the speaker needed to dispatch the UI update to the main thread anyway, incrementing the counter there, too, is indeed the best solution.
This is a very common scenario, where we dispatch both a model and UI update to the main queue. As long as you are judicious about limiting what (and how frequently) you dispatch to the main thread, you should be fine. But since we have to perform the UI update on the main thread, anyway, including the trivial model update there to eliminate any data race, too, is prudent.
So, performing a simple increment of a counter object is definitely suitable to perform on the main queue, especially because the speaker had to dispatch the UI update to the main thread, anyway. Introducing another synchronization mechanism in such a simple scenario would be an unnecessary complication.