kohesive / injekt

Dependency Injection for Kotlin
MIT License
235 stars 19 forks source link

addSingletonFactory may call its instantiation method multiple times #39

Open inorichi opened 6 years ago

inorichi commented 6 years ago

Hello, I've been a long time user of this library and I liked it because of its simplicity. I know it's now deprecated in favor of the other Kotlin's DI libraries but I don't feel the need to change it and I've been hit by this bug I think it's worth mentioning.

This only happens when injecting the instance from multiple threads and the initialization takes some time.

Example demostrating the issue:

class A {
    init {
        Log.w("TAG", "Init A")
        Thread.sleep(1000) // Simulate slow init
    }
}

In your InjektModule class:

override fun InjektRegistrar.registerInjectables() {
    ...

    addSingletonFactory { A() } 
}

Then, to cause multiple instances (example using coroutines):

fun main(args: Array<String>) {
    repeat(5) {
        async { Injekt.get<A>() }
    }
}

One proposed solution is to change the addSingletonFactory implementation to a lazy so that synchronization is taken into account:

@Suppress("UNCHECKED_CAST")
override fun <R: Any> addSingletonFactory(forType: TypeReference<R>, factoryCalledOnce: () -> R) {
    factories.put(forType.type, {
        (existingValues.getOrPut(Instance(forType.type, NOKEY)) { lazy { factoryCalledOnce() } } as Lazy<R>).value
    })
}