From f9620c1f128c4c6d76383f4a5a294473ab51ae5d Mon Sep 17 00:00:00 2001 From: Jenna Antilla <46546946+jennantilla@users.noreply.github.com> Date: Tue, 30 Jul 2024 11:24:26 -0700 Subject: [PATCH 1/3] Remove IAM from messages list if 410 is reached - Changed `messages` from an immutable list to a mutable list --- .../onesignal/inAppMessages/internal/InAppMessagesManager.kt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/OneSignalSDK/onesignal/in-app-messages/src/main/java/com/onesignal/inAppMessages/internal/InAppMessagesManager.kt b/OneSignalSDK/onesignal/in-app-messages/src/main/java/com/onesignal/inAppMessages/internal/InAppMessagesManager.kt index 6513e3f548..abaf6f6774 100644 --- a/OneSignalSDK/onesignal/in-app-messages/src/main/java/com/onesignal/inAppMessages/internal/InAppMessagesManager.kt +++ b/OneSignalSDK/onesignal/in-app-messages/src/main/java/com/onesignal/inAppMessages/internal/InAppMessagesManager.kt @@ -79,7 +79,7 @@ internal class InAppMessagesManager( // IAMs loaded remotely from on_session // If on_session won't be called this will be loaded from cache - private var messages: List = listOf() + private var messages: MutableList = mutableListOf() // IAMs that have been dismissed by the user // This mean they have already displayed to the user @@ -255,7 +255,7 @@ internal class InAppMessagesManager( val newMessages = _backend.listInAppMessages(appId, subscriptionId) if (newMessages != null) { - this.messages = newMessages + this.messages = newMessages as MutableList evaluateInAppMessages() } } @@ -390,6 +390,7 @@ internal class InAppMessagesManager( queueMessageForDisplay(messageToDisplay!!) } else if (result == false) { _state.inAppMessageIdShowing = null + messages.remove(messageToDisplay) messageWasDismissed(messageToDisplay!!, true) } } From 7e0e6bc2f391ea9543616e1473df54332676f1e7 Mon Sep 17 00:00:00 2001 From: Jenna Antilla <46546946+jennantilla@users.noreply.github.com> Date: Tue, 30 Jul 2024 11:24:41 -0700 Subject: [PATCH 2/3] Ensure thread-safe access to `messages` list - Added synchronization to `makeRedisplayMessagesAvailableWithTriggers` and `attemptToShowInAppMessage` to prevent concurrent modification issues - Refactored `evaluateInAppMessages` to collect messages for display inside a synchronized block and process them outside to avoid suspension points within critical sections --- .../internal/InAppMessagesManager.kt | 40 ++++++++++++------- 1 file changed, 26 insertions(+), 14 deletions(-) diff --git a/OneSignalSDK/onesignal/in-app-messages/src/main/java/com/onesignal/inAppMessages/internal/InAppMessagesManager.kt b/OneSignalSDK/onesignal/in-app-messages/src/main/java/com/onesignal/inAppMessages/internal/InAppMessagesManager.kt index abaf6f6774..5b850c9193 100644 --- a/OneSignalSDK/onesignal/in-app-messages/src/main/java/com/onesignal/inAppMessages/internal/InAppMessagesManager.kt +++ b/OneSignalSDK/onesignal/in-app-messages/src/main/java/com/onesignal/inAppMessages/internal/InAppMessagesManager.kt @@ -106,6 +106,8 @@ internal class InAppMessagesManager( private val fetchIAMMutex = Mutex() private var lastTimeFetchedIAMs: Long? = null + private val lock = Any() + override var paused: Boolean get() = _state.paused set(value) { @@ -265,16 +267,22 @@ internal class InAppMessagesManager( */ private suspend fun evaluateInAppMessages() { Logging.debug("InAppMessagesManager.evaluateInAppMessages()") - - for (message in messages) { - // Make trigger evaluation first, dynamic trigger might change "trigger changed" flag value for redisplay messages - if (_triggerController.evaluateMessageTriggers(message)) { - setDataForRedisplay(message) - if (!dismissedMessages.contains(message.messageId) && !message.isFinished) { - queueMessageForDisplay(message) + val messagesToQueue = mutableListOf() + + synchronized(lock) { + for (message in messages) { + if (_triggerController.evaluateMessageTriggers(message)) { + setDataForRedisplay(message) + if (!dismissedMessages.contains(message.messageId) && !message.isFinished) { + messagesToQueue.add(message) + } } } } + + for (message in messagesToQueue) { + queueMessageForDisplay(message) + } } /** @@ -458,13 +466,17 @@ internal class InAppMessagesManager( newTriggersKeys: Collection, isNewTriggerAdded: Boolean, ) { - for (message in messages) { - val isMessageDisplayed = redisplayedInAppMessages.contains(message) - val isTriggerOnMessage = _triggerController.isTriggerOnMessage(message, newTriggersKeys) - val isOnlyDynamicTriggers = _triggerController.messageHasOnlyDynamicTriggers(message) - if (!message.isTriggerChanged && isMessageDisplayed && (isTriggerOnMessage || isNewTriggerAdded && isOnlyDynamicTriggers)) { - Logging.debug("InAppMessagesManager.makeRedisplayMessagesAvailableWithTriggers: Trigger changed for message: $message") - message.isTriggerChanged = true + synchronized(lock) { + for (message in messages) { + val isMessageDisplayed = redisplayedInAppMessages.contains(message) + val isTriggerOnMessage = + _triggerController.isTriggerOnMessage(message, newTriggersKeys) + val isOnlyDynamicTriggers = + _triggerController.messageHasOnlyDynamicTriggers(message) + if (!message.isTriggerChanged && isMessageDisplayed && (isTriggerOnMessage || isNewTriggerAdded && isOnlyDynamicTriggers)) { + Logging.debug("InAppMessagesManager.makeRedisplayMessagesAvailableWithTriggers: Trigger changed for message: $message") + message.isTriggerChanged = true + } } } } From 5f5c4c0a646fb4d143bc91f2a850512d7d9060cf Mon Sep 17 00:00:00 2001 From: Jenna Antilla <46546946+jennantilla@users.noreply.github.com> Date: Fri, 2 Aug 2024 10:23:27 -0700 Subject: [PATCH 3/3] Refactor synchronization lock - Changed `synchronized(lock)` to `synchronized(messages)` --- .../inAppMessages/internal/InAppMessagesManager.kt | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/OneSignalSDK/onesignal/in-app-messages/src/main/java/com/onesignal/inAppMessages/internal/InAppMessagesManager.kt b/OneSignalSDK/onesignal/in-app-messages/src/main/java/com/onesignal/inAppMessages/internal/InAppMessagesManager.kt index 5b850c9193..4c3f1e5d88 100644 --- a/OneSignalSDK/onesignal/in-app-messages/src/main/java/com/onesignal/inAppMessages/internal/InAppMessagesManager.kt +++ b/OneSignalSDK/onesignal/in-app-messages/src/main/java/com/onesignal/inAppMessages/internal/InAppMessagesManager.kt @@ -106,8 +106,6 @@ internal class InAppMessagesManager( private val fetchIAMMutex = Mutex() private var lastTimeFetchedIAMs: Long? = null - private val lock = Any() - override var paused: Boolean get() = _state.paused set(value) { @@ -269,7 +267,7 @@ internal class InAppMessagesManager( Logging.debug("InAppMessagesManager.evaluateInAppMessages()") val messagesToQueue = mutableListOf() - synchronized(lock) { + synchronized(messages) { for (message in messages) { if (_triggerController.evaluateMessageTriggers(message)) { setDataForRedisplay(message) @@ -466,7 +464,7 @@ internal class InAppMessagesManager( newTriggersKeys: Collection, isNewTriggerAdded: Boolean, ) { - synchronized(lock) { + synchronized(messages) { for (message in messages) { val isMessageDisplayed = redisplayedInAppMessages.contains(message) val isTriggerOnMessage =