Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
Daeda88 opened this issue Nov 21, 2023 · 6 comments

Comments

@Daeda88
Copy link

Daeda88 commented Nov 21, 2023

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)
@russhwolf
Copy link
Contributor

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

@russhwolf
Copy link
Contributor

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
Copy link
Author

Daeda88 commented Nov 23, 2023

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
Copy link
Author

Daeda88 commented Nov 27, 2023

fwiw, the problem persists on Kotlin 1.9.21

@arnaudgiuliani
Copy link
Member

Thanks for the feedback. I see the PR

@arnaudgiuliani
Copy link
Member

see linked PR #1799

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants