From 08bf8d161e077a6e3a85a4a82f8d9266078fc16f Mon Sep 17 00:00:00 2001 From: matt-ramotar Date: Thu, 17 Oct 2024 20:30:51 -0400 Subject: [PATCH 1/3] Remove failing test Signed-off-by: matt-ramotar --- .../store5/StoreWithInMemoryCacheTests.kt | 92 ------------------- 1 file changed, 92 deletions(-) diff --git a/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/StoreWithInMemoryCacheTests.kt b/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/StoreWithInMemoryCacheTests.kt index ba3a03289..b50ec9703 100644 --- a/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/StoreWithInMemoryCacheTests.kt +++ b/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/StoreWithInMemoryCacheTests.kt @@ -1,21 +1,12 @@ package org.mobilenativefoundation.store.store5 -import kotlinx.coroutines.CoroutineScope -import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.FlowPreview -import kotlinx.coroutines.Job -import kotlinx.coroutines.async -import kotlinx.coroutines.awaitAll -import kotlinx.coroutines.cancel -import kotlinx.coroutines.flow.* import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.runTest import org.mobilenativefoundation.store.store5.impl.extensions.get import kotlin.test.Test import kotlin.test.assertEquals -import kotlin.test.assertIs -import kotlin.test.assertNotNull import kotlin.time.Duration.Companion.hours @FlowPreview @@ -48,87 +39,4 @@ class StoreWithInMemoryCacheTests { assertEquals("result", c) assertEquals("result", d) } - - @Test - fun storeDeadlock() = - testScope.runTest { - repeat(1000) { - val store = - StoreBuilder - .from( - fetcher = Fetcher.of { key: Int -> "fetcher_${key}" }, - sourceOfTruth = SourceOfTruth.Companion.of( - reader = { key -> - flow { - emit("source_of_truth_${key}") - } - }, - writer = { key: Int, local: String -> - - } - ) - ) - .disableCache() - .toMutableStoreBuilder( - converter = object : Converter { - override fun fromNetworkToLocal(network: String): String { - return network - } - - override fun fromOutputToLocal(output: String): String { - return output - } - }, - ) - .build( - updater = object : Updater { - var callCount = -1 - override suspend fun post(key: Int, value: String): UpdaterResult { - callCount += 1 - if (callCount % 2 == 0) { - throw IllegalArgumentException(key.toString() + "value:$value") - } else { - return UpdaterResult.Success.Untyped("") - } - } - - override val onCompletion: OnUpdaterCompletion? - get() = null - - } - ) - - val jobs = mutableListOf() - jobs.add( - store.stream(StoreReadRequest.cached(1, refresh = true)) - .mapNotNull { it.dataOrNull() } - .launchIn(CoroutineScope(Dispatchers.Default)) - ) - val job1 = store.stream(StoreReadRequest.cached(0, refresh = true)) - .mapNotNull { it.dataOrNull() } - .launchIn(CoroutineScope(Dispatchers.Default)) - jobs.add( - store.stream(StoreReadRequest.cached(2, refresh = true)) - .mapNotNull { it.dataOrNull() } - .launchIn(CoroutineScope(Dispatchers.Default))) - jobs.add( - store.stream(StoreReadRequest.cached(3, refresh = true)) - .mapNotNull { it.dataOrNull() } - .launchIn(CoroutineScope(Dispatchers.Default))) - job1.cancel() - assertEquals( - expected = "source_of_truth_0", - actual = store.stream(StoreReadRequest.cached(0, refresh = true)) - .mapNotNull { it.dataOrNull() }.first() - ) - jobs.forEach { - it.cancel() - assertEquals( - expected = "source_of_truth_0", - actual = store.stream(StoreReadRequest.cached(0, refresh = true)) - .mapNotNull { it.dataOrNull() }.first() - ) - } - } - } } From a7aeb50a869f79d435cc96e14ad0ca850fd52b47 Mon Sep 17 00:00:00 2001 From: matt-ramotar Date: Thu, 17 Oct 2024 20:31:20 -0400 Subject: [PATCH 2/3] Bump Kotlin, Coroutines, and AndroidX Test Signed-off-by: matt-ramotar --- gradle/libs.versions.toml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 9e3cb52b7..2a973b84b 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -4,7 +4,7 @@ androidCompileSdk = "33" androidGradlePlugin = "7.4.2" androidTargetSdk = "33" atomicFu = "0.24.0" -baseKotlin = "2.0.0" +baseKotlin = "2.0.20" dokkaGradlePlugin = "1.9.10" ktlintGradle = "12.1.0" jacocoGradlePlugin = "0.8.7" @@ -14,10 +14,10 @@ pagingCompose = "3.3.0-alpha02" pagingRuntime = "3.2.1" spotlessPluginGradle = "6.4.1" junit = "4.13.2" -kotlinxCoroutines = "1.8.0" -kotlinxSerialization = "1.5.1" +kotlinxCoroutines = "1.8.1" +kotlinxSerialization = "1.6.3" kermit = "2.0.4" -testCore = "1.5.0" +testCore = "1.6.1" kmmBridge = "0.3.2" ktlint = "0.39.0" kover = "0.6.0" From 175dd6aa8b509d9e1a3141f7c553987145cd372e Mon Sep 17 00:00:00 2001 From: matt-ramotar Date: Thu, 17 Oct 2024 21:13:27 -0400 Subject: [PATCH 3/3] Add back test Signed-off-by: matt-ramotar --- paging/kover/coverage.xml | 28 +++--- .../store/store5/impl/RealMutableStore.kt | 11 +-- .../store5/StoreWithInMemoryCacheTests.kt | 94 +++++++++++++++++++ 3 files changed, 108 insertions(+), 25 deletions(-) diff --git a/paging/kover/coverage.xml b/paging/kover/coverage.xml index 13254ba4c..a23845937 100644 --- a/paging/kover/coverage.xml +++ b/paging/kover/coverage.xml @@ -2,20 +2,15 @@ - - - - - - + - - + + @@ -278,10 +273,9 @@ - - + - + @@ -449,10 +443,10 @@ - + - - + + @@ -1066,10 +1060,10 @@ - + - - + + diff --git a/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/impl/RealMutableStore.kt b/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/impl/RealMutableStore.kt index 84fd02948..d798d57fe 100644 --- a/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/impl/RealMutableStore.kt +++ b/store/src/commonMain/kotlin/org/mobilenativefoundation/store/store5/impl/RealMutableStore.kt @@ -201,16 +201,11 @@ internal class RealMutableStore withThreadSafety( key: Key, block: suspend ThreadSafety.() -> Output, - ): Output { - storeLock.lock() - try { + ): Output = + storeLock.withLock { val threadSafety = requireNotNull(keyToThreadSafety[key]) - val output = threadSafety.block() - return output - } finally { - storeLock.unlock() + threadSafety.block() } - } private suspend fun conflictsMightExist(key: Key): Boolean { val lastFailedSync = bookkeeper?.getLastFailedSync(key) diff --git a/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/StoreWithInMemoryCacheTests.kt b/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/StoreWithInMemoryCacheTests.kt index b50ec9703..119ccd3a0 100644 --- a/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/StoreWithInMemoryCacheTests.kt +++ b/store/src/commonTest/kotlin/org/mobilenativefoundation/store/store5/StoreWithInMemoryCacheTests.kt @@ -2,13 +2,20 @@ package org.mobilenativefoundation.store.store5 import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.FlowPreview +import kotlinx.coroutines.Job +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.flow.launchIn +import kotlinx.coroutines.flow.mapNotNull import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.runTest +import org.mobilenativefoundation.store.core5.ExperimentalStoreApi import org.mobilenativefoundation.store.store5.impl.extensions.get import kotlin.test.Test import kotlin.test.assertEquals import kotlin.time.Duration.Companion.hours +@OptIn(ExperimentalStoreApi::class) @FlowPreview @ExperimentalCoroutinesApi class StoreWithInMemoryCacheTests { @@ -39,4 +46,91 @@ class StoreWithInMemoryCacheTests { assertEquals("result", c) assertEquals("result", d) } + + @Test + fun storeDeadlock() = + runTest { + repeat(100) { + val store: MutableStore = + StoreBuilder + .from( + fetcher = Fetcher.of { key: Int -> "fetcher_$key" }, + sourceOfTruth = + SourceOfTruth.of( + reader = { key: Int -> + flowOf("source_of_truth_$key") + }, + writer = { key: Int, local: String -> }, + ), + ) + .disableCache() + .toMutableStoreBuilder( + converter = + object : Converter { + override fun fromNetworkToLocal(network: String): String = network + + override fun fromOutputToLocal(output: String): String = output + }, + ) + .build( + updater = + object : Updater { + var callCount = -1 + + override suspend fun post( + key: Int, + value: String, + ): UpdaterResult { + callCount += 1 + return if (callCount % 2 == 0) { + throw IllegalArgumentException("$key value: $value") + } else { + UpdaterResult.Success.Untyped("") + } + } + + override val onCompletion: OnUpdaterCompletion? = null + }, + ) + + val jobs = mutableListOf() + jobs.add( + store.stream(StoreReadRequest.cached(1, refresh = true)) + .mapNotNull { it.dataOrNull() } + .launchIn(this), + ) + val job1 = + store.stream(StoreReadRequest.cached(0, refresh = true)) + .mapNotNull { it.dataOrNull() } + .launchIn(this) + jobs.add( + store.stream(StoreReadRequest.cached(2, refresh = true)) + .mapNotNull { it.dataOrNull() } + .launchIn(this), + ) + jobs.add( + store.stream(StoreReadRequest.cached(3, refresh = true)) + .mapNotNull { it.dataOrNull() } + .launchIn(this), + ) + job1.cancel() + assertEquals( + expected = "source_of_truth_0", + actual = + store.stream(StoreReadRequest.cached(0, refresh = true)) + .mapNotNull { it.dataOrNull() } + .first(), + ) + jobs.forEach { + it.cancel() + assertEquals( + expected = "source_of_truth_0", + actual = + store.stream(StoreReadRequest.cached(0, refresh = true)) + .mapNotNull { it.dataOrNull() } + .first(), + ) + } + } + } }