Merge pull request #1465 from npresseault/fix_race_scope

Fix race condition in scope resolveInstance
InsertKoinIO · Nov 14, 2022 · e472191 · e472191
2 parents ccef9a3 + 9f5ef3f
commit e472191
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 12 deletions.
diff --git a/core/koin-core/build.gradle b/core/koin-core/build.gradle
@@ -49,6 +49,7 @@ kotlin {
             dependencies {
                 implementation kotlin("test-common")
                 implementation kotlin("test-annotations-common")
+                implementation "org.jetbrains.kotlinx:kotlinx-coroutines-test:1.6.4"
             }
         }
 

diff --git a/core/koin-core/src/commonMain/kotlin/org/koin/core/scope/Scope.kt b/core/koin-core/src/commonMain/kotlin/org/koin/core/scope/Scope.kt
@@ -218,13 +218,17 @@ data class Scope(
         val parameters = parameterDef?.invoke()
         if (parameters != null) {
             _koin.logger.log(Level.DEBUG) { "| put parameters on stack $parameters " }
-            _parameterStack.addFirst(parameters)
+            KoinPlatformTools.synchronized(this@Scope) {
+                _parameterStack.addFirst(parameters)
+            }
         }
         val instanceContext = InstanceContext(_koin, this, parameters)
         val value = resolveValue<T>(qualifier, clazz, instanceContext, parameterDef)
         if (parameters != null) {
             _koin.logger.log(Level.DEBUG) { "| remove parameters from stack" }
-            _parameterStack.removeFirstOrNull()
+            KoinPlatformTools.synchronized(this@Scope) {
+                _parameterStack.removeFirstOrNull()
+            }
         }
         return value
     }
@@ -252,7 +256,9 @@ data class Scope(
             findInOtherScope<T>(clazz, qualifier, parameterDef)
         }
         ?: run {
-            _parameterStack.clear()
+            KoinPlatformTools.synchronized(this@Scope) {
+                _parameterStack.clear()
+            }
             _koin.logger.log(Level.DEBUG) { "| clear parameter stack" }
             throwDefinitionNotFound(qualifier, clazz)
         })

diff --git a/core/koin-core/src/commonTest/kotlin/org/koin/core/ParametersInjectionTest.kt b/core/koin-core/src/commonTest/kotlin/org/koin/core/ParametersInjectionTest.kt
@@ -1,13 +1,18 @@
 package org.koin.core
 
-import kotlin.test.assertEquals
-import kotlin.test.Test
+import kotlinx.coroutines.Dispatchers
+import kotlinx.coroutines.ExperimentalCoroutinesApi
+import kotlinx.coroutines.async
+import kotlinx.coroutines.awaitAll
+import kotlinx.coroutines.test.runTest
 import org.koin.Simple
 import org.koin.core.logger.Level
 import org.koin.core.parameter.parametersOf
 import org.koin.core.qualifier.named
 import org.koin.dsl.koinApplication
 import org.koin.dsl.module
+import kotlin.test.Test
+import kotlin.test.assertEquals
 import kotlin.test.assertNull
 
 class ParametersInjectionTest {
@@ -44,7 +49,7 @@ class ParametersInjectionTest {
         assertNull(a.id)
     }
 
-    internal class MyOptionalSingle(val i : Int, val o : String? = null)
+    internal class MyOptionalSingle(val i: Int, val o: String? = null)
 
     @Test
     fun nullable_injection_param_in_graph() {
@@ -53,15 +58,15 @@ class ParametersInjectionTest {
             printLogger(Level.DEBUG)
             modules(
                 module {
-                    single { p -> MyOptionalSingle(p.get(),getOrNull()) }
+                    single { p -> MyOptionalSingle(p.get(), getOrNull()) }
                 })
         }
 
         val koin = app.koin
         val value = 42
-        val a: MyOptionalSingle = koin.get { parametersOf(value)}
+        val a: MyOptionalSingle = koin.get { parametersOf(value) }
 
-        assertEquals(value,a.i)
+        assertEquals(value, a.i)
         assertNull(a.o)
     }
 
@@ -118,9 +123,9 @@ class ParametersInjectionTest {
         val app = koinApplication {
             printLogger(Level.DEBUG)
             modules(
-                    module {
-                        single { (i: Int?) -> Simple.MySingleWithNull(i) }
-                    })
+                module {
+                    single { (i: Int?) -> Simple.MySingleWithNull(i) }
+                })
         }
 
         val koin = app.koin
@@ -227,4 +232,28 @@ class ParametersInjectionTest {
         assertEquals(42, f.ints.id)
         assertEquals("42", f.strings.s)
     }
+
+    @Test
+    @OptIn(ExperimentalCoroutinesApi::class)
+    fun `inject across multiple threads`() = runTest {
+        val app = koinApplication {
+            modules(
+                module {
+                    factory { (i: Int) -> Simple.MyIntFactory(i) }
+                })
+        }
+
+        val koin = app.koin
+
+        repeat(1000) {
+            val range = (0 until 1000)
+            val deferreds = range.map {
+                async(Dispatchers.Default) {
+                    koin.get<Simple.MyIntFactory> { parametersOf(it) }
+                }
+            }
+            val values = awaitAll(*deferreds.toTypedArray())
+            assertEquals(range.map { it }, values.map { it.id })
+        }
+    }
 }