ANR Defensive Coding (#1132)

* Made a number of changes to try and improve background ANRs

Added some more logs to the BatchMessageReceiveJob (to make it easier to track a specific job)
Shifted the ConversationActivity adapter initialisation to run on a background thread to reduce the hang when opening a conversation
Updated the ConversationViewModel to cache the recipient and openGroup values to avoid accessing the database unnecessarily
Updated the code to just stop all current closed group pollers instead of fetching a list to stop
Updated the PN registration to be triggered in an AsyncTask
Updated the call code to unregister a couple of additional receivers
Updated the background poller so it waits for 15 mins before running and doesn't replace the existing scheduler (allows for PNs to trigger explicit background polling)
Fixed an issue where we were sending push notifications which were too large and likely to fail as a result (non-pre-offer call messages)
Fixed an issue where a failing Open Group poller could prevent the background poller from receiving and processing DMs

* Updated to a more coroutine-y convention
This commit is contained in:
Morgan Pretty
2023-03-31 13:24:36 +11:00
committed by GitHub
parent 5e28af2be4
commit eb739bdc9b
11 changed files with 269 additions and 73 deletions

View File

@@ -94,19 +94,19 @@ class BatchMessageReceiveJob(
} catch (e: Exception) {
when (e) {
is MessageReceiver.Error.DuplicateMessage, MessageReceiver.Error.SelfSend -> {
Log.i(TAG, "Couldn't receive message, failed with error: ${e.message}")
Log.i(TAG, "Couldn't receive message, failed with error: ${e.message} (id: $id)")
}
is MessageReceiver.Error -> {
if (!e.isRetryable) {
Log.e(TAG, "Couldn't receive message, failed permanently", e)
Log.e(TAG, "Couldn't receive message, failed permanently (id: $id)", e)
}
else {
Log.e(TAG, "Couldn't receive message, failed", e)
Log.e(TAG, "Couldn't receive message, failed (id: $id)", e)
failures += messageParameters
}
}
else -> {
Log.e(TAG, "Couldn't receive message, failed", e)
Log.e(TAG, "Couldn't receive message, failed (id: $id)", e)
failures += messageParameters
}
}
@@ -155,11 +155,11 @@ class BatchMessageReceiveJob(
else -> MessageReceiver.handle(message, proto, openGroupID)
}
} catch (e: Exception) {
Log.e(TAG, "Couldn't process message.", e)
Log.e(TAG, "Couldn't process message (id: $id)", e)
if (e is MessageReceiver.Error && !e.isRetryable) {
Log.e(TAG, "Message failed permanently",e)
Log.e(TAG, "Message failed permanently (id: $id)", e)
} else {
Log.e(TAG, "Message failed",e)
Log.e(TAG, "Message failed (id: $id)", e)
failures += parameters
}
}
@@ -196,12 +196,12 @@ class BatchMessageReceiveJob(
}
private fun handleSuccess(dispatcherName: String) {
Log.i(TAG, "Completed processing of ${messages.size} messages")
Log.i(TAG, "Completed processing of ${messages.size} messages (id: $id)")
this.delegate?.handleJobSucceeded(this, dispatcherName)
}
private fun handleFailure(dispatcherName: String) {
Log.i(TAG, "Handling failure of ${failures.size} messages (${messages.size - failures.size} processed successfully)")
Log.i(TAG, "Handling failure of ${failures.size} messages (${messages.size - failures.size} processed successfully) (id: $id)")
this.delegate?.handleJobFailed(this, dispatcherName, Exception("One or more jobs resulted in failure"))
}

View File

@@ -180,7 +180,20 @@ object MessageSender {
val hash = it["hash"] as? String
message.serverHash = hash
handleSuccessfulMessageSend(message, destination, isSyncMessage)
val shouldNotify = ((message is VisibleMessage || message is UnsendRequest || message is CallMessage) && !isSyncMessage)
val shouldNotify: Boolean = when (message) {
is VisibleMessage, is UnsendRequest -> !isSyncMessage
is CallMessage -> {
// Note: Other 'CallMessage' types are too big to send as push notifications
// so only send the 'preOffer' message as a notification
when (message.type) {
SignalServiceProtos.CallMessage.Type.PRE_OFFER -> true
else -> false
}
}
else -> false
}
/*
if (message is ClosedGroupControlMessage && message.kind is ClosedGroupControlMessage.Kind.New) {
shouldNotify = true

View File

@@ -54,10 +54,9 @@ class ClosedGroupPollerV2 {
setUpPolling(groupPublicKey)
}
fun stop() {
val storage = MessagingModuleConfiguration.shared.storage
val allGroupPublicKeys = storage.getAllClosedGroupPublicKeys()
allGroupPublicKeys.iterator().forEach { stopPolling(it) }
fun stopAll() {
futures.forEach { it.value.cancel(false) }
isPolling.forEach { isPolling[it.key] = false }
}
fun stopPolling(groupPublicKey: String) {

View File

@@ -26,6 +26,7 @@ import org.session.libsignal.utilities.ThreadUtils
import org.session.libsignal.utilities.recover
import org.session.libsignal.utilities.toHexString
import java.util.Date
import java.util.concurrent.atomic.AtomicReference
import kotlin.collections.set
private typealias Path = List<Snode>
@@ -43,13 +44,27 @@ object OnionRequestAPI {
private val snodeFailureCount = mutableMapOf<Snode, Int>()
var guardSnodes = setOf<Snode>()
var _paths: AtomicReference<List<Path>?> = AtomicReference(null)
var paths: List<Path> // Not a set to ensure we consistently show the same path to the user
get() = database.getOnionRequestPaths()
get() {
val paths = _paths.get()
if (paths != null) { return paths }
// Storing this in an atomic variable as it was causing a number of background
// ANRs when this value was accessed via the main thread after tapping on
// a notification)
val result = database.getOnionRequestPaths()
_paths.set(result)
return result
}
set(newValue) {
if (newValue.isEmpty()) {
database.clearOnionRequestPaths()
_paths.set(null)
} else {
database.setOnionRequestPaths(newValue)
_paths.set(newValue)
}
}