Re-fix #1898 by not unnessary blocking the main thread

* Reverted the fix back to 1.4.0 that executes background / foregrounding logic on the main thread instead of on the replicator thread which could block the main thread. * Make CBLRestReplicator’s _active an atomic property. * In `-backgroundTaskExpired` set `_deepBackground = YES` on the main thread instead of on the replicator thread as it has been done in 1.4.0.
couchbase · Dec 22, 2017 · 491aca5 · 491aca5
1 parent 86c6365
commit 491aca5
Show file tree

Hide file tree

Showing 4 changed files with 46 additions and 45 deletions.
diff --git a/Source/CBLRestReplicator+Backgrounding.m b/Source/CBLRestReplicator+Backgrounding.m
@@ -65,66 +65,63 @@ - (void) endBackgrounding {
 
 // Called when the replicator goes idle (from -updateActive)
 - (void) okToEndBackgrounding {
-    if ([_bgMonitor hasBackgroundTask]) {
-        _deepBackground = YES;
-        [self updateSuspended];
-        LogTo(Sync, @"%@: Now idle; ending background task", self);
-        [_bgMonitor endBackgroundTask];  // will probably suspend the process immediately
-    }
-}
-
-- (void) updateSuspended {
-    BOOL suspended = (_filesystemUnavailable || _deepBackground);
-    self.suspended = suspended;
+    dispatch_async(dispatch_get_main_queue(), ^{
+        if ([_bgMonitor hasBackgroundTask]) {
+            _deepBackground = YES;
+            [self updateSuspended];
+            LogTo(Sync, @"%@: Now idle; ending background task", self);
+            [_bgMonitor endBackgroundTask];  // will probably suspend the process immediately
+        }
+    });
 }
 
 
 ////// All the methods below are called on the MAIN THREAD, not the replicator thread ////////
 
 
 - (void) appBackgrounding {
-    [self.db doSync: ^{
-        if (_active && [_bgMonitor beginBackgroundTaskNamed: self.description]) {
-            LogTo(Sync, @"%@: App backgrounding; starting temporary background task", self);
-        } else {
-            LogTo(Sync, @"%@: App backgrounding, but replication is inactive; suspending", self);
-            _deepBackground = YES;
-            [self updateSuspended];
-        }
-    }];
+    if (self.active && [_bgMonitor beginBackgroundTaskNamed: self.description]) {
+        LogTo(Sync, @"%@: App backgrounding; starting temporary background task", self);
+    } else {
+        LogTo(Sync, @"%@: App backgrounding, but can't run background task; suspending", self);
+        _deepBackground = YES;
+        [self updateSuspended];
+    }
 }
 
 
 - (void) appForegrounding {
-    [self.db doSync: ^{
-        BOOL ended = [_bgMonitor endBackgroundTask];
-        if (ended)
-            LogTo(Sync, @"%@: App foregrounded, ending background task", self);
-        if (_deepBackground) {
-            _deepBackground = NO;
-            [self updateSuspended];
-        }
-    }];
+    BOOL ended = [_bgMonitor endBackgroundTask];
+    if (ended)
+        LogTo(Sync, @"%@: App foregrounded, ending background task", self);
+    if (_deepBackground) {
+        _deepBackground = NO;
+        [self updateSuspended];
+    }
 }
 
 
 // Called if process runs out of background time before replication finishes.
 // Must do its work synchronously, before the OS quits the app.
 - (void) backgroundTaskExpired {
-    [self.db doSync: ^{
-        LogTo(Sync, @"%@: Background task time expired!", self);
-        _deepBackground = YES;
-        [self updateSuspended];
-    }];
+    LogTo(Sync, @"%@: Background task time expired!", self);
+    _deepBackground = YES;
+    [self updateSuspended];
 }
 
 
 // Called when the app is about to lose access to files:
 - (void) fileAccessChanged: (NSNotification*)n {
+    LogTo(Sync, @"%@: Device locked, database unavailable", self);
+    _filesystemUnavailable = [n.name isEqual: UIApplicationProtectedDataWillBecomeUnavailable];
+    [self updateSuspended];
+}
+
+
+- (void) updateSuspended {
+    BOOL suspended = (_filesystemUnavailable || _deepBackground);
     [self.db doSync: ^{
-        LogTo(Sync, @"%@: Device locked, database unavailable", self);
-        _filesystemUnavailable = [n.name isEqual: UIApplicationProtectedDataWillBecomeUnavailable];
-        [self updateSuspended];
+        self.suspended = suspended;
     }];
 }
 

diff --git a/Source/CBLRestReplicator+Internal.h b/Source/CBLRestReplicator+Internal.h
@@ -15,7 +15,7 @@
 @interface CBLRestReplicator ()
 {
     @protected
-    BOOL _running, _online, _active;
+    BOOL _running, _online;
     CBL_ReplicatorSettings* _settings;
     CBLDatabase* __weak _db;
     NSString* _lastSequence;

diff --git a/Source/CBLRestReplicator.h b/Source/CBLRestReplicator.h
@@ -14,8 +14,10 @@
 /** Abstract base class for push or pull replications. */
 @interface CBLRestReplicator : NSObject <CBL_Replicator>
 
+@property (readonly, atomic) BOOL active; // for backgrounding and unit tests
+
 #if DEBUG
-@property (readonly) BOOL running, active; // for unit tests
+@property (readonly) BOOL running; // for unit tests
 #endif
 
 @end
diff --git a/Source/CBLRestReplicator.m b/Source/CBLRestReplicator.m
@@ -57,6 +57,7 @@ - (void) okToEndBackgrounding;
 @interface CBLRestReplicator () <CBLRemoteRequestDelegate>
 @property (readwrite) CBL_ReplicatorStatus status;
 @property (readwrite, copy) NSDictionary* remoteCheckpoint;
+@property (readwrite, atomic) BOOL active;
 - (void) updateActive;
 - (void) fetchRemoteCheckpointDoc;
 - (void) saveLastSequence;
@@ -85,8 +86,9 @@ @implementation CBLRestReplicator
 }
 
 @synthesize db=_db, settings=_settings, serverCert=_serverCert;
+@synthesize active=_active;
 #if DEBUG
-@synthesize running=_running, active=_active;
+@synthesize running=_running;
 #endif
 
 
@@ -314,10 +316,6 @@ - (void) start {
     [self updateStatus];
     _startTime = CFAbsoluteTimeGetCurrent();
 
-#if TARGET_OS_IPHONE
-    [self setupBackgrounding];
-#endif
-
     if (!_settings.continuous || [NSClassFromString(@"CBL_URLProtocol") handlesURL: _settings.remote]) {
         [self goOnline];    // non-continuous or local-to-local replication
     } else {
@@ -334,6 +332,10 @@ - (void) start {
         [_host startOnRunLoop: CFRunLoopGetCurrent()];
         [self reachabilityChanged: _host];
     }
+
+#if TARGET_OS_IPHONE
+    [self setupBackgrounding];
+#endif
 }
 
 
@@ -521,7 +523,7 @@ - (void) updateActive {
         }
 
         LogTo(Sync, @"%@ Progress: set active = %d", self, active);
-        _active = active;
+        self.active = active;
         [self updateStatus];
         [self postProgressChanged];
         if (!_active) {