瀏覽代碼

call maybeNetwork() from non-ui thread

B. Petersen 4 年之前
父節點
當前提交
d8b2c8c4af
共有 2 個文件被更改,包括 69 次插入14 次删除
  1. 38 13
      deltachat-ios/AppDelegate.swift
  2. 31 1
      docs/uiview-layout.md

+ 38 - 13
deltachat-ios/AppDelegate.swift

@@ -65,8 +65,12 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UNUserNotificationCenterD
         setStockTranslations()
 
         reachability.whenReachable = { reachability in
+            // maybeNetwork() shall not be called in ui thread;
+            // Reachability::reachabilityChanged uses DispatchQueue.main.async only
             logger.info("network: reachable", reachability.connection.description)
-            self.dcContext.maybeNetwork()
+            DispatchQueue.global(qos: .background).async { [weak self] in
+                self?.dcContext.maybeNetwork()
+            }
         }
 
         reachability.whenUnreachable = { _ in
@@ -118,6 +122,14 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UNUserNotificationCenterD
         logger.info("---- background-fetch ----")
         increaseDebugCounter("notify-local-wakeup")
 
+        // if we're in foreground, there is nothing to do
+        if self.appIsInForeground {
+            logger.warning("➡️ app already in foreground")
+            completionHandler(.newData)
+            return
+        }
+
+        // we're in background, run IO for a little time
         dcContext.maybeStartIo()
         dcContext.maybeNetwork()
 
@@ -138,20 +150,24 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UNUserNotificationCenterD
         logger.info("---- foreground ----")
         appIsInForeground = true
         dcContext.maybeStartIo()
-        if reachability.connection != .none {
-            self.dcContext.maybeNetwork()
-        }
 
-        if let userDefaults = UserDefaults.shared, userDefaults.bool(forKey: UserDefaults.hasExtensionAttemptedToSend) {
-            userDefaults.removeObject(forKey: UserDefaults.hasExtensionAttemptedToSend)
-            let nc = NotificationCenter.default
+        DispatchQueue.global(qos: .background).async { [weak self] in
+            guard let self = self else { return }
+            if self.appIsInForeground {
+                if self.reachability.connection != .none {
+                    self.dcContext.maybeNetwork()
+                }
 
-            DispatchQueue.main.async {
-                nc.post(
-                    name: dcNotificationChanged,
-                    object: nil,
-                    userInfo: [:]
-                )
+                if let userDefaults = UserDefaults.shared, userDefaults.bool(forKey: UserDefaults.hasExtensionAttemptedToSend) {
+                    userDefaults.removeObject(forKey: UserDefaults.hasExtensionAttemptedToSend)
+                    DispatchQueue.main.async {
+                        NotificationCenter.default.post(
+                            name: dcNotificationChanged,
+                            object: nil,
+                            userInfo: [:]
+                        )
+                    }
+                }
             }
         }
     }
@@ -390,6 +406,15 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UNUserNotificationCenterD
         logger.verbose("Notifications: didReceiveRemoteNotification \(userInfo)")
         increaseDebugCounter("notify-remote-receive")
 
+        // didReceiveRemoteNotification might be called if we're in foreground,
+        // in this case, there is no need to wait for things or do sth.
+        if self.appIsInForeground {
+            logger.warning("➡️ app already in foreground")
+            completionHandler(.newData)
+            return
+        }
+
+        // we're in background, run IO for a little time
         dcContext.maybeStartIo()
         dcContext.maybeNetwork()
 

+ 31 - 1
docs/uiview-layout.md

@@ -1,4 +1,4 @@
-# UIView layout thoughts
+# code layout thoughts
 
 ## view definitions
   
@@ -116,6 +116,31 @@ a tricky part (see eg. [3]) seems to be to hold the correct type of rerences to
   https://developer.apple.com/tutorials/swiftui/interfacing-with-uikit
 
 
+## notification system
+
+- `NotificationCenter.default.post()`
+  post events to all observers _synchronously_ on the same thread;
+  this may not be the thread the observer registered itself [5].
+  Observers may receive the event in any random thread therefore.
+
+- best practise [6] would be that the observers assume,
+  they receive the notification in any thread
+  and call `DispatchQueue.main.async` where actually needed.
+  for new code, we should use that approach, it won't hurt.
+
+- however, existing code might assume
+  that the notification arrives in main thread;
+  therefore, we also call `DispatchQueue.main.async` for sending events out.
+
+- using always main thread for sending/receiving and adding/remove observers
+  also avoids other multithreading-issues of NotificationCenter itself,
+  see https://lapcatsoftware.com/articles/nsnotificationcenter-is-threadsafe-not.html
+  for details.
+
+tl;dr: post from main thread and also add/remove observers from main thread.
+on receiving, assume, things will block.
+
+
 ## some sources
 
 [1] https://developer.apple.com/documentation/uikit/uiapplication
@@ -126,3 +151,8 @@ a tricky part (see eg. [3]) seems to be to hold the correct type of rerences to
 https://github.com/deltachat/deltachat-ios/issues/323
   
 [4] https://developer.apple.com/documentation/uikit/uiapplicationdelegate/1622921-application
+
+[5] https://developer.apple.com/library/archive/documentation/Cocoa/Conceptual/Notifications/Articles/Threading.html#//apple_ref/doc/uid/20001289-CEGJFDFG
+
+[6] https://medium.com/@hadhi631/myths-and-facts-about-nsnotifcation-posting-and-receiving-df7f5729b19f
+