koin icon indicating copy to clipboard operation
koin copied to clipboard

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

Open Daeda88 opened this issue 1 year ago • 5 comments

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:

  • koin-core:3.5.0 (also confirmed on 3.5.2-RC1)

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)

Daeda88 avatar Nov 21 '23 11:11 Daeda88

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

russhwolf avatar Nov 23 '23 02:11 russhwolf

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

russhwolf avatar Nov 23 '23 03:11 russhwolf

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 avatar Nov 23 '23 05:11 Daeda88

fwiw, the problem persists on Kotlin 1.9.21

Daeda88 avatar Nov 27 '23 10:11 Daeda88

Thanks for the feedback. I see the PR

arnaudgiuliani avatar Jan 25 '24 13:01 arnaudgiuliani

see linked PR #1799

arnaudgiuliani avatar Feb 29 '24 18:02 arnaudgiuliani