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

Generated modules use global state and break context isolation. #102

Closed
Jadarma opened this issue Nov 30, 2023 · 4 comments
Closed

Generated modules use global state and break context isolation. #102

Jadarma opened this issue Nov 30, 2023 · 4 comments

Comments

@Jadarma
Copy link

Jadarma commented Nov 30, 2023

Describe the bug
Module code generation forces instantiating the module class twice and breaks context isolation.
When using annotations, it is impossible to run Koin in parallel, such as in the case of unit / integration tests, as the same one global module will be shared across Koin instances.

To Reproduce
Consider the following module:

@Module
class MyModule {

    @Named("Seed")
    @Single(createdAtStart = true)
    fun randomSeed() = SecureRandom.getInstanceStrong().nextLong()
}

Which I then install in a Ktor Application:

fun Application.configureDI() {
    val moduleInstance = MyModule()

    install(Koin) {
        modules(moduleInstance.module)
    }

    val applicationInstance = this
    val koinInstance = applicationInstance.getKoin()
    val seedInstance = koinInstance.get<Long>(named("Seed"))

    println("Ktor $applicationInstance, uses koin $koinInstance, and module $moduleInstance has seed $seedInstance")
}

If I run two Ktor test applications in parallel, I get the following logged:

Ktor io.ktor.server.application.Application@2e7d7e03 uses koin org.koin.core.Koin@5b54ae0e, and module com.example.MyModule@66e53ab8 has seed -4596009551291032851
Ktor io.ktor.server.application.Application@3a3a4af6 uses koin org.koin.core.Koin@2efa5d9d, and module com.example.MyModule@6ceeaad3 has seed -4596009551291032851

So, two different Ktor instances, with two different Koin instances, with two different module instances, ended up generating the same seed... hmm.

Looking at the generated code, I see:

public val io_example_MyModule : Module = module {
	val moduleInstance = io.example.MyModule()
	single(qualifier=org.koin.core.qualifier.StringQualifier("Seed"),createdAtStart=true) { moduleInstance.randomSeed() } bind(kotlin.Long::class)
}
public val io.example.MyModule.module : org.koin.core.module.Module get() = io_example_MyModule

Even though the module is an extension over an already created instance of MyModule, it resolves to a global variable which instantiates it itself once more, and that instance is then used to provide the values, regardless of module or Koin instance they were defined in.

Expected behavior
The .module extension should get a module based on the instance it was called on instead of global state.

Koin project used and used version:
io.insert-koin:koin-bom:3.5.1
io.insert-koin:koin-annotations-bom:1.3.0

Extra notes:
This is a more trivial example for reproducibility, how I originally found this bug is that I was trying to execute parallel integration tests with different databases, but I got constraint violation exceptions, which made no sense unless the same connection was reused. I was able to trace back the culprit to this. If I instead declare the module with the normal DSL, then the issue goes away.

@arnaudgiuliani
Copy link
Member

yes, this is clearly a limitation in the current design. It needs to be followed-up for next milestone, as it's an important change

Copy link

stale bot commented Jun 28, 2024

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the status:wontfix This will not be worked on label Jun 28, 2024
@Jadarma
Copy link
Author

Jadarma commented Jun 29, 2024

Bad stale bot, bad!

@stale stale bot removed the status:wontfix This will not be worked on label Jun 29, 2024
@arnaudgiuliani
Copy link
Member

Fixed in #136

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

No branches or pull requests

2 participants