Swift - Application crash when using two different OperationQueues with KVO

Question

I'm getting two type of information with JSON and I'm adding "operations" to 2 different Operation Queues Classes with addObserver(forKeyPath:"operations"...). In the function observeValue I'm checking if operationQueue1.operations.isEmpty and then I refresh my information in UI. I'm doing the same thing with if else with operationQueue2, but when the 2 operations are started in sometime the application crash with error message: *** Terminating app due to uncaught exception 'NSRangeException', reason: 'Cannot remove an observer <AppName.ViewController 0x102977800> for the key path "operations" from <AppName.OperationQueue1 0x1c4a233c0> because it is not registered as an observer.' I don't have problem when only 1 operation is started. Any suggestions?

func getInfo1(){//runned in viewDidLoad
  operationQueue1.addObserver(forKeyPath:"operations"...)
  operationQueue1.dataTask(URL:"..."....){
      DispatchQueue.main.async{
  NotificationCenter.default.postNotification(NSNotification.Name(rawValue: "NewDataReceived1", userInfo:infoFromTheWebsite)
      }
  }
}

func NewDataReceived1(){
  here I add the information to arrays to be loaded in tableView1
}

HERE IS THE CODE FOR 2ND INFO WHICH IS THE SAME

override func observeValue(forKeyPath keyPath: String?, ....){
        if(object as? operationQueue1 == operationQueue1Class && keyPath == "operations" && context == context1){
             if(operationQueue1.operations.isEmpty){
                  DispatchQueue.main.async{
                        operationQueue1..removeObserver(self, forKeyPath:"operations")
                        Timer.scheduled("refreshingTableInformation1") 
                   }
             }
        }else if(operationQueue2....){
             SAME AS OPERATION 1, BUT USING DIFFERENT FUNC TO REFRESH TABLE INFORMATION AND THE TABLES ARE DIFFERENT
        }else{
            super.observeValue(forKeyPath: keyPath, of: object, change: change, context: context)
        }
}

func refreshingTableInformation1(){
     tableView1.reloadData()
     Timer.scheduled("getInfo1", repeat:false)
}

func refreshingTableInformation2(){
     tableView2.reloadData()
     Timer.scheduled("getInfo2", repeat:false)
}

Sometimes it works 10 secs and crash and sometimes works for more than 60 seconds and then crash...

Answer 1

Your code here is vulnerable to race conditions. Consider the following scenario:

1. getInfo1() is called, which adds an operation to operationQueue1.

2. The operation completes, which means your KVO observation is called. The queue is now empty, so your observation schedules removal of your observer on the main dispatch queue.

3. Now, before the operation you've submitted to the main queue is able to run, something else calls getInfo1(), which adds a new operation to operationQueue1, which completes before the operation you queued in step 2 has had the chance to run (hey, maybe the main queue was busy with something; it's easy for this to happen since it's a serial queue).

4. Your observation for the first call of getInfo1() is called again while the queue is empty, causing another deregister block to be submitted to the main queue.

5. The two deregister blocks finally get to execute on the main queue. The second one crashes the program, since you've already deregistered your observer.

You could probably fix this problem (assuming the code doesn't have more problems of this nature) by using Swift 4's block-based observers instead, and setting the observer to nil instead of explicitly deregistering it. However, I propose that KVO is the wrong tool for what you're trying to do. As the instructions for the old "Crystal Quest" game used to say, it's a bit like using an anti-aircraft gun to kill a mosquito.

From what I can see from the code above, it looks like you're using KVO just to schedule a notification for when an operation or a group of operations you submit to the queue finishes. Depending on what your dataTask method actually does, here's what I'd do instead:

• If you submit only one operation: set the operation's completionBlock property to a closure that refreshes your table information.

• If you submit more than one operation: Make a new BlockOperation which refreshes your table information, and call addDependency on that operation with every other operation you submit to the queue. Then, submit that operation.

This will provide you with a much cleaner and more trouble-free way of monitoring completion for your tasks. And since you don't need the queue to completely empty anymore, you might not even have to use two separate queues anymore, depending on what else you're doing with them.