Merge pull request #265 from IFTTT/bugfix/fixing_scheduler_race_condition

Fixing a race condition in the scheduler

Author: ssathy2

IFTTT SDK/ConnectionsMonitor.swift

@@ -112,11 +112,19 @@ class ConnectionsMonitor: SynchronizationSubscriber {
 
 /// Defines a cancellable `Operation` subclass to allow for cancellable network requests.
 private class CancellableNetworkOperation: Operation {
+    /// The current in-flight network request
     private var task: URLSessionDataTask? = nil
+    
+    /// The completion closure to invoke once the request is completed
     private let completion: ConnectionNetworkController.ConnectionResponseClosure
+    
+    /// The network controller to perform requests with
     private weak var networkController: ConnectionNetworkController?
+    
+    /// The request to perform
     private let request: Connection.Request
 
+    /// Creates an instance of `CancellableNetworkOperation`.
     init(networkController: ConnectionNetworkController,
          request: Connection.Request,
          _ completion: @escaping ConnectionNetworkController.ConnectionResponseClosure) {
@@ -125,17 +133,83 @@ private class CancellableNetworkOperation: Operation {
         self.completion = completion
     }
 
+    /// State for this operation.
+    @objc private enum OperationState: Int {
+        case ready
+        case executing
+        case finished
+    }
+    
+    /// Concurrent queue for synchronizing access to `state`.
+    private let stateQueue = DispatchQueue(label: "com.ifttt.connections_monitor.rw.state", attributes: .concurrent)
+    
+    /// Private backing stored property for `state`.
+    private var _state: OperationState = .ready
+    
+    /// The state of the operation
+    @objc private dynamic var state: OperationState {
+        get {
+            return stateQueue.sync { _state }
+        }
+        set {
+            stateQueue.async(flags: .barrier) { self._state = newValue }
+        }
+    }
+    
+    override var isReady: Bool {
+        return state == .ready && super.isReady
+    }
+    
+    override var isExecuting: Bool {
+        return state == .executing
+    }
+    
+    override var isFinished: Bool {
+        return state == .finished
+    }
+    
+    override var isAsynchronous: Bool {
+        return true
+    }
+
+    open override class func keyPathsForValuesAffectingValue(forKey key: String) -> Set<String> {
+        let keys = ["isReady", "isFinished", "isExecuting"]
+        if keys.contains(key) {
+            return .init(arrayLiteral: #keyPath(state))
+        }
+        
+        return super.keyPathsForValuesAffectingValue(forKey: key)
+    }
+    
+    override func start() {
+        if isCancelled {
+            state = .finished
+            return
+        }
+        
+        state = .executing
+        
+        main()
+    }
+    
     override func main() {
-        task = networkController?.start(request: request, completion: { response in
+        task = networkController?.start(request: request, completion: { [weak self] response in
+            guard let self = self else { return }
             self.completion(response)
-            self.completionBlock?()
+            self.finish()
         })
         task?.resume()
     }
 
     override func cancel() {
-        super.cancel()
         task?.cancel()
+        finish()
     }
 
+    /// Call this method to finish the operation
+    private func finish() {
+        if !isFinished {
+            state = .finished
+        }
+    }
 }

IFTTT SDK/ConnectionsSynchronizer.swift

@@ -106,7 +106,7 @@ final class ConnectionsSynchronizer {
                                        eventPublisher: eventPublisher)
 
         let connectionsMonitor = ConnectionsMonitor(connectionsRegistry: connectionsRegistry)
-        let nativeServicesCoordinator = NativeServicesCoordinator(locationService: location)
+        let nativeServicesCoordinator = NativeServicesCoordinator(locationService: location, permissionsRequestor: permissionsRequestor)
 
         self.subscribers = [
             connectionsMonitor,
@@ -184,8 +184,6 @@ final class ConnectionsSynchronizer {
 
     /// Call this to stop the synchronization completely. Safe to be called multiple times.
     private func stop() {
-        if state == .stopped { return }
-        
         stopNotifications()
         Keychain.resetIfNecessary(force: true)
         scheduler.stop()
@@ -265,15 +263,18 @@ final class ConnectionsSynchronizer {
 /// Handles coordination of native services with a set of connections
 private class NativeServicesCoordinator {
     private let locationService: LocationService
+    private let permissionsRequestor: PermissionsRequestor
     private let operationQueue: OperationQueue
 
-    init(locationService: LocationService) {
+    init(locationService: LocationService, permissionsRequestor: PermissionsRequestor) {
         self.locationService = locationService
+        self.permissionsRequestor = permissionsRequestor
         self.operationQueue = OperationQueue.main
     }
 
     func processConnectionUpdate(_ updates: Set<Connection.ConnectionStorage>) {
         operationQueue.addOperation {
+            self.permissionsRequestor.processUpdate(with: updates)
             self.locationService.updateRegions(from: updates)
         }
     }

IFTTT SDK/Keychain.swift

@@ -95,7 +95,7 @@ final class Keychain {
     }
 
     private class func reset() {
-        Key.AllCases().forEach {
+        Key.allCases.forEach {
             removeValue(for: $0)
         }
     }

IFTTT SDK/PermissionsRequestor.swift

@@ -24,7 +24,7 @@ final class PermissionsRequestor: SynchronizationSubscriber {
         self.registry = registry
     }
 
-    private func processUpdate(with connections: Set<Connection.ConnectionStorage>) {
+    func processUpdate(with connections: Set<Connection.ConnectionStorage>) {
         let operations = connections.reduce(.init()) { (currSet, connections) -> Set<Trigger> in
             return currSet.union(connections.allTriggers)
         }

IFTTTConnectSDK.podspec

@@ -1,6 +1,6 @@
 Pod::Spec.new do |spec|
   spec.name             = "IFTTTConnectSDK"
-  spec.version          = "2.5.8"
+  spec.version          = "2.5.9"
   spec.summary          = "Allows your users to activate programmable IFTTT Connections directly in your app."
   spec.description      = <<-DESC
   - Easily authenticate your services to IFTTT through the Connect Button