InsertKoinIO / koin

Koin - a pragmatic lightweight dependency injection framework for Kotlin & Kotlin Multiplatform
https://insert-koin.io
Apache License 2.0
8.98k stars 710 forks source link

ScopeRegistry.closeAllScopes() causes ConcurrentModificationException for iOS on Kotlin 1.9.20 #1711

Closed Daeda88 closed 7 months ago

Daeda88 commented 10 months ago

Describe the bug After upgrading to Kotlin 1.9.20, stopKoin() causes a ConcurrentModificationException on iOS when multiple scopes are added.

This is because stopKoin calls closeAllScopes iterates over alls scopes and calls scope.close() which calls _koin.scopeRegistry.deleteScope(this) which modifies the hashmap being iterated over.

Only seeing this problem on iOS and only as of Kotlin 1.9.20 so I suspect something changed on the expect/actuals of the Hashmap there. Regardless, this should not crash

To Reproduce On iOS, create a few custom scopes and then stop Koin.

Expected behavior StopKoin should close all scopes without issue

Koin module and version:

Snippet or Sample project to help reproduce

class TestKoin {

    @Test
    fun testKoin() {
        val koin = startKoin {  }
        koin.koin.createScope(
            "Test-${KoinPlatformTools.generateId()}",
            TypeQualifier(String::class)
        )
        stopKoin()
    }

}

throws

kotlin.ConcurrentModificationException
    at kotlin.Exception#<init>(/opt/buildAgent/work/2fed3917837e7e79/kotlin/kotlin-native/runtime/src/main/kotlin/kotlin/Exceptions.kt:25)
    at kotlin.RuntimeException#<init>(/opt/buildAgent/work/2fed3917837e7e79/kotlin/kotlin-native/runtime/src/main/kotlin/kotlin/Exceptions.kt:36)
    at kotlin.ConcurrentModificationException#<init>(/opt/buildAgent/work/2fed3917837e7e79/kotlin/kotlin-native/runtime/src/main/kotlin/kotlin/Exceptions.kt:176)
    at kotlin.ConcurrentModificationException#<init>(/opt/buildAgent/work/2fed3917837e7e79/kotlin/kotlin-native/runtime/src/main/kotlin/kotlin/Exceptions.kt:178)
    at kotlin.collections.HashMap.Itr#checkForComodification(/opt/buildAgent/work/2fed3917837e7e79/kotlin/libraries/stdlib/native-wasm/src/kotlin/collections/HashMap.kt:587)
    at kotlin.collections.HashMap.ValuesItr#next(/opt/buildAgent/work/2fed3917837e7e79/kotlin/libraries/stdlib/native-wasm/src/kotlin/collections/HashMap.kt:605)
    at kotlin.collections.Iterator#next(/opt/buildAgent/work/2fed3917837e7e79/kotlin/libraries/stdlib/native-wasm/src/kotlin/collections/Iterator.kt:18)
    at org.koin.core.registry.ScopeRegistry.closeAllScopes#internal(/Users/runner/work/koin/koin/core/koin-core/src/commonMain/kotlin/org/koin/core/registry/ScopeRegistry.kt:92)
    at org.koin.core.registry.ScopeRegistry#close(/Users/runner/work/koin/koin/core/koin-core/src/commonMain/kotlin/org/koin/core/registry/ScopeRegistry.kt:86)
    at org.koin.core.Koin#close(/Users/runner/work/koin/koin/core/koin-core/src/commonMain/kotlin/org/koin/core/Koin.kt:297)
    at org.koin.core.context.MutableGlobalContext#stopKoin(/Users/runner/work/koin/koin/core/koin-core/src/nativeMain/kotlin/org/koin/core/context/GlobalContext.kt)
    at org.koin.core.context.KoinContext#stopKoin(/Users/runner/work/koin/koin/core/koin-core/src/commonMain/kotlin/org/koin/core/context/KoinContext.kt:43)
    at org.koin.core.context#stopKoin(/Users/runner/work/koin/koin/core/koin-core/src/commonMain/kotlin/org/koin/core/context/DefaultContextExt.kt:45)
    at <global>.TestKoin#testKoin(/Users/gijsvanveen/AndroidStudioProjects/TestKoin/shared/src/commonTest/kotlin/TestKoin.kt:17)
    at $TestKoin$test$0.$testKoin$FUNCTION_REFERENCE$1.invoke#internal(/Users/gijsvanveen/AndroidStudioProjects/TestKoin/shared/src/commonTest/kotlin/TestKoin.kt:11)
    at $TestKoin$test$0.$testKoin$FUNCTION_REFERENCE$1.$<bridge-UNNN>invoke(/Users/gijsvanveen/AndroidStudioProjects/TestKoin/shared/src/commonTest/kotlin/TestKoin.kt:11)
    at kotlin.Function1#invoke(/Users/gijsvanveen/.gradle/daemon/8.4/[K][Suspend]Functions:1)
    at kotlin.native.internal.test.BaseClassSuite.TestCase#run(/opt/buildAgent/work/2fed3917837e7e79/kotlin/kotlin-native/runtime/src/main/kotlin/kotlin/native/internal/test/TestSuite.kt:92)
    at kotlin.native.internal.test.TestCase#run(/opt/buildAgent/work/2fed3917837e7e79/kotlin/kotlin-native/runtime/src/main/kotlin/kotlin/native/internal/test/TestSuite.kt:19)
    at kotlin.native.internal.test.TestRunner.run#internal(/opt/buildAgent/work/2fed3917837e7e79/kotlin/kotlin-native/runtime/src/main/kotlin/kotlin/native/internal/test/TestRunner.kt:248)
    at kotlin.native.internal.test.TestRunner.runIteration#internal(/opt/buildAgent/work/2fed3917837e7e79/kotlin/kotlin-native/runtime/src/main/kotlin/kotlin/native/internal/test/TestRunner.kt:274)
    at kotlin.native.internal.test.TestRunner#run(/opt/buildAgent/work/2fed3917837e7e79/kotlin/kotlin-native/runtime/src/main/kotlin/kotlin/native/internal/test/TestRunner.kt:289)
    at kotlin.native.internal.test#testLauncherEntryPoint(/opt/buildAgent/work/2fed3917837e7e79/kotlin/kotlin-native/runtime/src/main/kotlin/kotlin/native/internal/test/Launcher.kt:33)
    at kotlin.native.internal.test#main(/opt/buildAgent/work/2fed3917837e7e79/kotlin/kotlin-native/runtime/src/main/kotlin/kotlin/native/internal/test/Launcher.kt:38)
    at <global>.Konan_start(/opt/buildAgent/work/2fed3917837e7e79/kotlin/kotlin-native/runtime/src/main/kotlin/kotlin/native/internal/test/Launcher.kt:37)
    at <global>.Init_and_run_start(Unknown Source)
    at <global>.0x0(Unknown Source)
    at <global>.0x0(Unknown Source)
    at <global>.0x0(Unknown Source)
russhwolf commented 10 months ago

This also impacts checkModules() which internally calls koin.close() and runs through the same closeAllScopes() logic

russhwolf commented 10 months ago

I can see this as far back as Koin 3.1.x (which is the earliest I checked) when using Kotlin 1.9.20. I think that makes a good argument for this being a Kotlin regression, but I haven't tested against other Kotlin versions. I just came across it when updating an old project from Kotlin 1.5.x

Daeda88 commented 10 months ago

The Koin iOS implementation uses the kotlin 'HashMap' on iOS (android uses the Java 'ConcurrentHashMap') . Before that didn't crash, and while it's not great that it does now, this is more in line with what a regular Java 'HashMap' would do. So I get why JetBrains made this change. The solution is fairly simple: either make a custom 'ConcurrentHashMap' for Koin, or just copy the map into a local var on 'closeAllScopes()' and loop over that.

Daeda88 commented 10 months ago

fwiw, the problem persists on Kotlin 1.9.21

arnaudgiuliani commented 8 months ago

Thanks for the feedback. I see the PR

arnaudgiuliani commented 7 months ago

see linked PR #1799