From b2efb2248abde21916f666415f0d4c0f82244a59 Mon Sep 17 00:00:00 2001 From: Nicolas Presseault <618592+npresseault@users.noreply.github.com> Date: Wed, 2 Nov 2022 17:34:46 -0400 Subject: [PATCH 1/2] Fix an issue where concurrent calls to koin.get would create a race condition resulting in this stack trace: Fatal Exception: java.util.NoSuchElementException ArrayDeque is empty. kotlin.collections.ArrayDeque.removeFirst (ArrayDeque.kt:145) kotlin.collections.ArrayDeque.removeFirstOrNull (ArrayDeque.java:157) org.koin.core.scope.Scope.resolveInstance (Scope.kt:244) org.koin.core.scope.Scope.get (Scope.kt:204) org.koin.core.scope.Scope.getOrNull (Scope.kt:172) ArrayDeque is not a thread-safe collection, by wrapping calls to parameterStack in a synchronized block it makes sure the race condition cannot happen. This fixes https://github.com/InsertKoinIO/koin/issues/1149 --- core/koin-core/build.gradle | 1 + .../kotlin/org/koin/core/scope/Scope.kt | 12 +++-- .../org/koin/core/ParametersInjectionTest.kt | 47 +++++++++++++++---- 3 files changed, 48 insertions(+), 12 deletions(-) diff --git a/core/koin-core/build.gradle b/core/koin-core/build.gradle index 4224c61f2..c9bf1f48d 100644 --- 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 index e305035a5..b9e5c69c5 100644 --- 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(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(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 index 122e2c3c5..8a22a287b 100644 --- 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 + + for (i in 0..1000) { + val range = (0 until 1000) + val deferreds = range.map { + async(Dispatchers.Default) { + koin.get { parametersOf(it) } + } + } + val values = awaitAll(*deferreds.toTypedArray()) + assertEquals(range.map { it }, values.map { it.id }) + } + } } \ No newline at end of file From 9f5ef3f75056ee63e556aca300f45115bbc15f83 Mon Sep 17 00:00:00 2001 From: Nicolas Presseault <618592+npresseault@users.noreply.github.com> Date: Thu, 3 Nov 2022 09:10:36 -0400 Subject: [PATCH 2/2] Cosmetic changes --- .../commonTest/kotlin/org/koin/core/ParametersInjectionTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 index 8a22a287b..1fbe6a0d6 100644 --- a/core/koin-core/src/commonTest/kotlin/org/koin/core/ParametersInjectionTest.kt +++ b/core/koin-core/src/commonTest/kotlin/org/koin/core/ParametersInjectionTest.kt @@ -245,7 +245,7 @@ class ParametersInjectionTest { val koin = app.koin - for (i in 0..1000) { + repeat(1000) { val range = (0 until 1000) val deferreds = range.map { async(Dispatchers.Default) {