From 70c0df1cf7112f20df806671750cf942ee238a89 Mon Sep 17 00:00:00 2001
From: Kirill Kamakin <k.kamakin@protonmail.ch>
Date: Sat, 20 Nov 2021 21:13:26 +0300
Subject: [PATCH] Fix ConcurrentModificationException in
 RecipePagingSourceFactory

It seems that it is possible to launch several coroutines on same
main thread of application. That's why it is possible to launch both
invoke and invalidate at the same time even though they are marked as
synchronized. To fix the issue this commit uses a concurrent collection
instead of synchronization.
---
 .../recipes/impl/RecipePagingSourceFactory.kt | 17 +++++++--
 .../impl/RecipePagingSourceFactoryTest.kt     | 35 +++++++++++++++++++
 2 files changed, 49 insertions(+), 3 deletions(-)
 create mode 100644 app/src/test/java/gq/kirmanak/mealient/data/recipes/impl/RecipePagingSourceFactoryTest.kt

diff --git a/app/src/main/java/gq/kirmanak/mealient/data/recipes/impl/RecipePagingSourceFactory.kt b/app/src/main/java/gq/kirmanak/mealient/data/recipes/impl/RecipePagingSourceFactory.kt
index a3c14e6..0e7499d 100644
--- a/app/src/main/java/gq/kirmanak/mealient/data/recipes/impl/RecipePagingSourceFactory.kt
+++ b/app/src/main/java/gq/kirmanak/mealient/data/recipes/impl/RecipePagingSourceFactory.kt
@@ -4,6 +4,7 @@ import androidx.paging.PagingSource
 import gq.kirmanak.mealient.data.recipes.db.RecipeStorage
 import gq.kirmanak.mealient.data.recipes.db.entity.RecipeSummaryEntity
 import timber.log.Timber
+import java.util.concurrent.ConcurrentSkipListSet
 import javax.inject.Inject
 import javax.inject.Singleton
 
@@ -11,9 +12,9 @@ import javax.inject.Singleton
 class RecipePagingSourceFactory @Inject constructor(
     private val recipeStorage: RecipeStorage
 ) : () -> PagingSource<Int, RecipeSummaryEntity> {
-    private val sources: MutableList<PagingSource<Int, RecipeSummaryEntity>> = mutableListOf()
+    private val sources: MutableSet<PagingSource<Int, RecipeSummaryEntity>> =
+        ConcurrentSkipListSet(PagingSourceComparator)
 
-    @Synchronized
     override fun invoke(): PagingSource<Int, RecipeSummaryEntity> {
         Timber.v("invoke() called")
         val newSource = recipeStorage.queryRecipes()
@@ -21,7 +22,6 @@ class RecipePagingSourceFactory @Inject constructor(
         return newSource
     }
 
-    @Synchronized
     fun invalidate() {
         Timber.v("invalidate() called")
         for (source in sources) {
@@ -31,4 +31,15 @@ class RecipePagingSourceFactory @Inject constructor(
         }
         sources.removeAll { it.invalid }
     }
+
+    private object PagingSourceComparator : Comparator<PagingSource<Int, RecipeSummaryEntity>> {
+        override fun compare(
+            left: PagingSource<Int, RecipeSummaryEntity>?,
+            right: PagingSource<Int, RecipeSummaryEntity>?
+        ): Int {
+            val leftHash = left?.hashCode() ?: 0
+            val rightHash = right?.hashCode() ?: 0
+            return leftHash - rightHash
+        }
+    }
 }
\ No newline at end of file
diff --git a/app/src/test/java/gq/kirmanak/mealient/data/recipes/impl/RecipePagingSourceFactoryTest.kt b/app/src/test/java/gq/kirmanak/mealient/data/recipes/impl/RecipePagingSourceFactoryTest.kt
new file mode 100644
index 0000000..8c98a32
--- /dev/null
+++ b/app/src/test/java/gq/kirmanak/mealient/data/recipes/impl/RecipePagingSourceFactoryTest.kt
@@ -0,0 +1,35 @@
+package gq.kirmanak.mealient.data.recipes.impl
+
+import dagger.hilt.android.testing.HiltAndroidTest
+import gq.kirmanak.mealient.data.recipes.db.RecipeStorage
+import gq.kirmanak.mealient.test.HiltRobolectricTest
+import kotlinx.coroutines.*
+import org.junit.Before
+import org.junit.Test
+import javax.inject.Inject
+
+@ExperimentalCoroutinesApi
+@HiltAndroidTest
+class RecipePagingSourceFactoryTest : HiltRobolectricTest() {
+    @Inject
+    lateinit var storage: RecipeStorage
+    lateinit var subject: RecipePagingSourceFactory
+
+    @Before
+    fun setUp() {
+        subject = RecipePagingSourceFactory(storage)
+    }
+
+    @Test
+    fun `when modifying concurrently then doesn't throw`(): Unit = runBlocking {
+        (0..100).map {
+            async(Dispatchers.Default) {
+                for (i in 0..100) {
+                    subject.invalidate()
+                    subject.invoke()
+                }
+            }
+        }.awaitAll()
+    }
+
+}
\ No newline at end of file