Move callback calls out of the synchronized blocks

OneSignal · Dec 15, 2023 · c9d5452 · c9d5452
1 parent b84f261
commit c9d5452
Show file tree

Hide file tree

Showing 3 changed files with 20 additions and 18 deletions.
diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/modeling/Model.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/modeling/Model.kt
@@ -436,9 +436,8 @@ open class Model(
         tag: String = ModelChangeTags.NORMAL,
         forceChange: Boolean = false,
     ) {
+        val oldValue = data[name]
         synchronized(data) {
-            val oldValue = data[name]
-
             if (oldValue == value && !forceChange) {
                 return
             }
@@ -448,9 +447,9 @@ open class Model(
             } else if (data.containsKey(name)) {
                 data.remove(name)
             }
-
-            notifyChanged(name, name, tag, oldValue, value)
         }
+
+        notifyChanged(name, name, tag, oldValue, value)
     }
 
     /**

diff --git a/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/modeling/ModelStore.kt b/OneSignalSDK/onesignal/core/src/main/java/com/onesignal/common/modeling/ModelStore.kt
@@ -89,9 +89,9 @@ abstract class ModelStore<TModel>(
     ) {
         synchronized(models) {
             persist()
-
-            changeSubscription.fire { it.onModelUpdated(args, tag) }
         }
+
+        changeSubscription.fire { it.onModelUpdated(args, tag) }
     }
 
     override fun replaceAll(
@@ -108,17 +108,18 @@ abstract class ModelStore<TModel>(
     }
 
     override fun clear(tag: String) {
+        val localList = models.toList()
+
         synchronized(models) {
-            val localList = models.toList()
             models.clear()
 
             persist()
+        }
 
-            for (item in localList) {
-                // no longer listen for changes to this model
-                item.unsubscribe(this)
-                changeSubscription.fire { it.onModelRemoved(item, tag) }
-            }
+        for (item in localList) {
+            // no longer listen for changes to this model
+            item.unsubscribe(this)
+            changeSubscription.fire { it.onModelRemoved(item, tag) }
         }
     }
 
@@ -138,9 +139,9 @@ abstract class ModelStore<TModel>(
             model.subscribe(this)
 
             persist()
-
-            changeSubscription.fire { it.onModelAdded(model, tag) }
         }
+
+        changeSubscription.fire { it.onModelAdded(model, tag) }
     }
 
     private fun removeItem(
@@ -154,9 +155,9 @@ abstract class ModelStore<TModel>(
             model.unsubscribe(this)
 
             persist()
-
-            changeSubscription.fire { it.onModelRemoved(model, tag) }
         }
+
+        changeSubscription.fire { it.onModelRemoved(model, tag) }
     }
 
     protected fun load() {

diff --git a/...gnalSDK/onesignal/core/src/main/java/com/onesignal/common/modeling/SingletonModelStore.kt b/...gnalSDK/onesignal/core/src/main/java/com/onesignal/common/modeling/SingletonModelStore.kt
@@ -37,12 +37,14 @@ open class SingletonModelStore<TModel>(
         model: TModel,
         tag: String,
     ) {
+        val existingModel = this.model
+
         synchronized(replaceLock) {
-            val existingModel = this.model
             existingModel.initializeFromModel(singletonId, model)
             store.persist()
-            changeSubscription.fire { it.onModelReplaced(existingModel, tag) }
         }
+
+        changeSubscription.fire { it.onModelReplaced(existingModel, tag) }
     }
 
     override fun subscribe(handler: ISingletonModelStoreChangeHandler<TModel>) = changeSubscription.subscribe(handler)