Merge pull request groue#1196 from layoutSubviews/development

Fix a crash when an observation is quickly cancelled

Author: groue

GRDB/ValueObservation/ValueConcurrentObserver.swift

@@ -192,7 +192,8 @@ extension ValueConcurrentObserver {
             (self.notificationCallbacks, self.databaseAccess)
         }
         guard let notificationCallbacks = notificationCallbacksOpt, let databaseAccess = databaseAccessOpt else {
-            // Likely a GRDB bug
+            // Likely a GRDB bug: during a synchronous start, user is not
+            // able to cancel observation.
             fatalError("can't start a cancelled or failed observation")
         }
 

GRDB/ValueObservation/ValueWriteOnlyObserver.swift

@@ -155,7 +155,8 @@ extension ValueWriteOnlyObserver {
             (self.notificationCallbacks, self.databaseAccess)
         }
         guard let notificationCallbacks = notificationCallbacksOpt, let writer = databaseAccessOpt?.writer else {
-            // Likely a GRDB bug
+            // Likely a GRDB bug: during a synchronous start, user is not
+            // able to cancel observation.
             fatalError("can't start a cancelled or failed observation")
         }
 
@@ -198,7 +199,11 @@ extension ValueWriteOnlyObserver {
         // from a database access.
         try writer.unsafeReentrantWrite { db in
             // Fetch & Start observing the database
-            let fetchedValue = try fetchAndStartObservation(db)
+            guard let fetchedValue = try fetchAndStartObservation(db) else {
+                // Likely a GRDB bug: during a synchronous start, user is not
+                // able to cancel observation.
+                fatalError("can't start a cancelled or failed observation")
+            }
 
             // Reduce
             return reduceQueue.sync {
@@ -218,12 +223,11 @@ extension ValueWriteOnlyObserver {
         // Start from a write access, so that self can register as a
         // transaction observer.
         writer.asyncWriteWithoutTransaction { db in
-            let isNotifying = self.lock.synchronized { self.notificationCallbacks != nil }
-            guard isNotifying else { return /* Cancelled */ }
-            
             do {
                 // Fetch & Start observing the database
-                let fetchedValue = try self.fetchAndStartObservation(db)
+                guard let fetchedValue = try self.fetchAndStartObservation(db) else {
+                    return /* Cancelled */
+                }
 
                 // Reduce
                 //
@@ -257,17 +261,19 @@ extension ValueWriteOnlyObserver {
 
     /// Fetches the initial value, and start observing the database.
     ///
+    /// Returns nil if the observation was cancelled before database observation
+    /// could start.
+    ///
     /// By grouping the initial fetch and the beginning of observation in a
     /// single database access, we are sure that no concurrent write can happen
     /// during the initial fetch, and that we won't miss any future change.
-    private func fetchAndStartObservation(_ db: Database) throws -> Reducer.Fetched {
+    private func fetchAndStartObservation(_ db: Database) throws -> Reducer.Fetched? {
         // TODO: [SR-214] remove -Opt suffix when we only support Xcode 12.5.1+
         let (eventsOpt, fetchOpt) = lock.synchronized {
             (notificationCallbacks?.events, databaseAccess?.fetch)
         }
         guard let events = eventsOpt, let fetch = fetchOpt else {
-            // Likely a GRDB bug
-            fatalError("can't start a cancelled or failed observation")
+            return nil /* Cancelled */
         }
 
         switch trackingMode {