From 544121d035d701c400ceb2d09183655edc0c661c Mon Sep 17 00:00:00 2001
From: Greyson Parrelli <greyson@signal.org>
Date: Mon, 10 Apr 2023 09:42:28 -0400
Subject: [PATCH] Minimize lock window in IncomingMessageObserver.

In particular, this was done to avoid a possible deadlock that could
occur between the IncomingMessageObserver lock and the JobManager lock.
---
 .../messages/IncomingMessageObserver.kt       | 52 +++++++++++--------
 1 file changed, 30 insertions(+), 22 deletions(-)

diff --git a/app/src/main/java/org/thoughtcrime/securesms/messages/IncomingMessageObserver.kt b/app/src/main/java/org/thoughtcrime/securesms/messages/IncomingMessageObserver.kt
index 651edb5c48..b47856e3d4 100644
--- a/app/src/main/java/org/thoughtcrime/securesms/messages/IncomingMessageObserver.kt
+++ b/app/src/main/java/org/thoughtcrime/securesms/messages/IncomingMessageObserver.kt
@@ -11,6 +11,7 @@ import android.net.ConnectivityManager
 import android.os.IBinder
 import androidx.annotation.VisibleForTesting
 import androidx.core.app.NotificationCompat
+import kotlinx.collections.immutable.toImmutableSet
 import org.signal.core.util.ThreadUtil
 import org.signal.core.util.concurrent.SignalExecutors
 import org.signal.core.util.logging.Log
@@ -36,7 +37,6 @@ import org.thoughtcrime.securesms.messages.protocol.BufferedProtocolStore
 import org.thoughtcrime.securesms.notifications.NotificationChannels
 import org.thoughtcrime.securesms.recipients.RecipientId
 import org.thoughtcrime.securesms.util.AppForegroundObserver
-import org.thoughtcrime.securesms.util.Util
 import org.whispersystems.signalservice.api.push.ServiceId
 import org.whispersystems.signalservice.api.util.UuidUtil
 import org.whispersystems.signalservice.api.websocket.WebSocketConnectionState
@@ -176,32 +176,40 @@ class IncomingMessageObserver(private val context: Application) {
   }
 
   private fun isConnectionNecessary(): Boolean {
-    lock.withLock {
-      val registered = SignalStore.account().isRegistered
-      val fcmEnabled = SignalStore.account().fcmEnabled
-      val hasNetwork = NetworkConstraint.isMet(context)
-      val hasProxy = SignalStore.proxy().isProxyEnabled
-      val forceWebsocket = SignalStore.internalValues().isWebsocketModeForced
-      val keepAliveCutoffTime = System.currentTimeMillis() - KEEP_ALIVE_TOKEN_MAX_AGE
-      val timeIdle = if (appVisible) 0 else System.currentTimeMillis() - lastInteractionTime
-      val removedRequests = keepAliveTokens.entries.removeIf { (_, createTime) -> createTime < keepAliveCutoffTime }
-      val decryptQueueEmpty = ApplicationDependencies.getJobManager().isQueueEmpty(PushDecryptMessageJob.QUEUE)
+    val timeIdle: Long
+    val keepAliveEntries: Set<Map.Entry<String, Long>>
+    val appVisibleSnapshot: Boolean
 
-      if (removedRequests) {
+    lock.withLock {
+      appVisibleSnapshot = appVisible
+      timeIdle = if (appVisibleSnapshot) 0 else System.currentTimeMillis() - lastInteractionTime
+
+      val keepAliveCutoffTime = System.currentTimeMillis() - KEEP_ALIVE_TOKEN_MAX_AGE
+      val removedKeepAliveToken = keepAliveTokens.entries.removeIf { (_, createTime) -> createTime < keepAliveCutoffTime }
+      if (removedKeepAliveToken) {
         Log.d(TAG, "Removed old keep web socket open requests.")
       }
 
-      val lastInteractionString = if (appVisible) "N/A" else timeIdle.toString() + " ms (" + (if (timeIdle < MAX_BACKGROUND_TIME) "within limit" else "over limit") + ")"
-      val conclusion = registered &&
-        (appVisible || timeIdle < MAX_BACKGROUND_TIME || !fcmEnabled || Util.hasItems(keepAliveTokens)) &&
-        hasNetwork &&
-        decryptQueueEmpty
-
-      val needsConnectionString = if (conclusion) "Needs Connection" else "Does Not Need Connection"
-
-      Log.d(TAG, "[$needsConnectionString] Network: $hasNetwork, Foreground: $appVisible, Time Since Last Interaction: $lastInteractionString, FCM: $fcmEnabled, Stay open requests: ${keepAliveTokens.entries}, Registered: $registered, Proxy: $hasProxy, Force websocket: $forceWebsocket, Decrypt Queue Empty: $decryptQueueEmpty")
-      return conclusion
+      keepAliveEntries = keepAliveTokens.entries.toImmutableSet()
     }
+
+    val registered = SignalStore.account().isRegistered
+    val fcmEnabled = SignalStore.account().fcmEnabled
+    val hasNetwork = NetworkConstraint.isMet(context)
+    val hasProxy = SignalStore.proxy().isProxyEnabled
+    val forceWebsocket = SignalStore.internalValues().isWebsocketModeForced
+    val decryptQueueEmpty = ApplicationDependencies.getJobManager().isQueueEmpty(PushDecryptMessageJob.QUEUE)
+
+    val lastInteractionString = if (appVisibleSnapshot) "N/A" else timeIdle.toString() + " ms (" + (if (timeIdle < MAX_BACKGROUND_TIME) "within limit" else "over limit") + ")"
+    val conclusion = registered &&
+      (appVisibleSnapshot || timeIdle < MAX_BACKGROUND_TIME || !fcmEnabled || keepAliveEntries.isNotEmpty()) &&
+      hasNetwork &&
+      decryptQueueEmpty
+
+    val needsConnectionString = if (conclusion) "Needs Connection" else "Does Not Need Connection"
+
+    Log.d(TAG, "[$needsConnectionString] Network: $hasNetwork, Foreground: $appVisibleSnapshot, Time Since Last Interaction: $lastInteractionString, FCM: $fcmEnabled, Stay open requests: $keepAliveEntries, Registered: $registered, Proxy: $hasProxy, Force websocket: $forceWebsocket, Decrypt Queue Empty: $decryptQueueEmpty")
+    return conclusion
   }
 
   private fun waitForConnectionNecessary() {