Fixing race condition in Scope

InsertKoinIO · Apr 18, 2023 · 1811bb6 · 1811bb6
1 parent 53898e3
commit 1811bb6
Show file tree

Hide file tree

Showing 7 changed files with 28 additions and 16 deletions.
diff --git a/core/koin-core/build.gradle b/core/koin-core/build.gradle
@@ -71,6 +71,7 @@ kotlin {
             dependsOn commonMain
             dependencies {
                 implementation 'org.jetbrains.kotlin:kotlin-stdlib-js'
+                implementation "co.touchlab:stately-concurrency:1.2.2"
             }
         }
 

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
@@ -32,6 +32,7 @@ import org.koin.ext.getFullName
 import org.koin.mp.KoinPlatformTimeTools
 import org.koin.mp.KoinPlatformTools
 import org.koin.mp.Lockable
+import org.koin.mp.ThreadLocal
 import kotlin.reflect.KClass
 
 @OptIn(KoinInternalApi::class)
@@ -56,7 +57,7 @@ data class Scope(
     private val _callbacks = arrayListOf<ScopeCallback>()
 
     @KoinInternalApi
-    val _parameterStack = ArrayDeque<ParametersHolder>()
+    val _parameterStackLocal = ThreadLocal<ArrayDeque<ParametersHolder>>()
 
     private var _closed: Boolean = false
     val logger: Logger get() = _koin.logger
@@ -221,19 +222,17 @@ data class Scope(
             throw ClosedScopeException("Scope '$id' is closed")
         }
         val parameters = parameterDef?.invoke()
+        var localDeque: ArrayDeque<ParametersHolder>? = null
         if (parameters != null) {
             _koin.logger.log(Level.DEBUG){ "| >> parameters $parameters " }
-            KoinPlatformTools.synchronized(this@Scope) {
-                _parameterStack.addFirst(parameters)
-            }
+            localDeque = _parameterStackLocal.get() ?: ArrayDeque<ParametersHolder>().also(_parameterStackLocal::set)
+            localDeque.addFirst(parameters)
         }
         val instanceContext = InstanceContext(_koin.logger, this, parameters)
         val value = resolveValue<T>(qualifier, clazz, instanceContext, parameterDef)
-        if (parameters != null) {
+        if (localDeque != null) {
             _koin.logger.debug("| << parameters")
-            KoinPlatformTools.synchronized(this@Scope) {
-                _parameterStack.removeFirstOrNull()
-            }
+            localDeque.removeFirstOrNull()
         }
         return value
     }
@@ -246,7 +245,7 @@ data class Scope(
     ) = (_koin.instanceRegistry.resolveInstance(qualifier, clazz, this.scopeQualifier, instanceContext)
         ?: run {
             _koin.logger.debug("|- ? t:'${clazz.getFullName()}' - q:'$qualifier' look in injected parameters")
-            _parameterStack.firstOrNull()?.getOrNull<T>(clazz)
+            _parameterStackLocal.get()?.firstOrNull()?.getOrNull<T>(clazz)
         }
         ?: run {
             _koin.logger.debug("|- ? t:'${clazz.getFullName()}' - q:'$qualifier' look at scope source" )
@@ -261,9 +260,7 @@ data class Scope(
             findInOtherScope<T>(clazz, qualifier, parameterDef)
         }
         ?: run {
-            KoinPlatformTools.synchronized(this@Scope) {
-                _parameterStack.clear()
-            }
+            _parameterStackLocal.remove()
             _koin.logger.debug("|- << parameters" )
             throwDefinitionNotFound(qualifier, clazz)
         })

diff --git a/core/koin-core/src/commonMain/kotlin/org/koin/mp/KoinPlatformTools.kt b/core/koin-core/src/commonMain/kotlin/org/koin/mp/KoinPlatformTools.kt
@@ -31,4 +31,10 @@ expect object KoinPlatformTools {
     fun <K, V> safeHashMap(): MutableMap<K, V>
 }
 
-expect open class Lockable()
+expect open class Lockable()
+
+expect open class ThreadLocal<T>() {
+    fun get(): T?
+    fun set(value: T?)
+    fun remove()
+}
diff --git a/core/koin-core/src/commonTest/kotlin/org/koin/core/ParameterStackTest.kt b/core/koin-core/src/commonTest/kotlin/org/koin/core/ParameterStackTest.kt
@@ -28,6 +28,6 @@ class ParameterStackTest {
             koin.get<Simple.MyStringFactory> { parametersOf(KoinPlatformTools.generateId()) }
         }
 
-        assertTrue(koin.scopeRegistry.rootScope._parameterStack.isEmpty())
+        assertTrue(koin.scopeRegistry.rootScope._parameterStackLocal.get()!!.isEmpty())
     }
 }
diff --git a/core/koin-core/src/jsMain/kotlin/org/koin/mp/PlatformTools.kt b/core/koin-core/src/jsMain/kotlin/org/koin/mp/PlatformTools.kt
@@ -1,5 +1,6 @@
 package org.koin.mp
 
+import co.touchlab.stately.concurrency.ThreadLocalRef
 import org.koin.core.context.GlobalContext
 import org.koin.core.context.KoinContext
 import org.koin.core.logger.*
@@ -36,4 +37,6 @@ actual object KoinPlatformTools {
     actual fun <K, V> safeHashMap(): MutableMap<K, V> = HashMap()
 }
 
-actual typealias Lockable = Any
+actual typealias Lockable = Any
+
+actual typealias ThreadLocal<T> = ThreadLocalRef<T>
diff --git a/core/koin-core/src/jvmMain/kotlin/org/koin/mp/KoinPlatformTools.kt b/core/koin-core/src/jvmMain/kotlin/org/koin/mp/KoinPlatformTools.kt
@@ -21,4 +21,6 @@ actual object KoinPlatformTools {
     actual fun <K, V> safeHashMap(): MutableMap<K, V> = ConcurrentHashMap<K, V>()
 }
 
-actual typealias Lockable = Any
+actual typealias Lockable = Any
+
+actual typealias ThreadLocal<T> = java.lang.ThreadLocal<T>
diff --git a/core/koin-core/src/nativeMain/kotlin/org/koin/mp/KoinPlatformTools.kt b/core/koin-core/src/nativeMain/kotlin/org/koin/mp/KoinPlatformTools.kt
@@ -2,6 +2,7 @@ package org.koin.mp
 
 import co.touchlab.stately.concurrency.Lock
 import co.touchlab.stately.concurrency.withLock
+import co.touchlab.stately.concurrency.ThreadLocalRef
 import org.koin.core.context.KoinContext
 import org.koin.core.context.globalContextByMemoryModel
 import org.koin.core.logger.Level
@@ -39,3 +40,5 @@ actual object KoinPlatformTools {
 actual open class Lockable {
     internal val lock = Lock()
 }
+
+actual typealias ThreadLocal<T> = ThreadLocalRef<T>