Skip to content

[core] Ensure onClose callbacks are invoked in reversed initialization order #2020

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.

Sign up for GitHub

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

Open
shchuko wants to merge 1 commit into
base: main
Choose a base branch
from
+229 −12
Open

[core] Ensure onClose callbacks are invoked in reversed initialization order #2020

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 28 additions & 2 deletions projects/core/koin-core/src/commonMain/kotlin/org/koin/core/instance/InstanceFactory.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@

package org.koin.core.instance

import co.touchlab.stately.concurrency.AtomicBoolean
import co.touchlab.stately.concurrency.AtomicLong
import org.koin.core.definition.BeanDefinition
import org.koin.core.error.InstanceCreationException
import org.koin.core.parameter.ParametersHolder
Expand All @@ -28,8 +30,12 @@ import org.koin.mp.Lockable
/**
* Koin Instance Holder
* create/get/release an instance of given definition
*
* Implements [Comparable] to provide factories soring by instance creation order
*/
abstract class InstanceFactory<T>(val beanDefinition: BeanDefinition<T>) : Lockable() {
abstract class InstanceFactory<T>(val beanDefinition: BeanDefinition<T>) : Lockable(), Comparable<InstanceFactory<*>> {
private val instanceCreationOrderSet = AtomicBoolean(false)
private var instanceCreationOrderPosition = Long.MAX_VALUE

/**
* Retrieve an instance
Expand All @@ -47,17 +53,36 @@ abstract class InstanceFactory<T>(val beanDefinition: BeanDefinition<T>) : Locka
context.logger.debug("| (+) '$beanDefinition'")
try {
val parameters: ParametersHolder = context.parameters ?: emptyParametersHolder()
return beanDefinition.definition.invoke(
val instance = beanDefinition.definition.invoke(
context.scope,
parameters,
)
if (instanceCreationOrderSet.compareAndSet(false, true)) {
instanceCreationOrderPosition = INSTANCE_CREATION_ORDER_COUNTER.incrementAndGet()

// just to be sure that counter provides normal values
check(instanceCreationOrderPosition >= 0) {
"Unexpected negative instance creation order position"
}

// it's required to have `Long.MAX_VALUE` instantiations happened to make this check fail,
// which is not likely to happen
check(instanceCreationOrderPosition < Long.MAX_VALUE) {
"Instance creation order position reached Long.MAX_VALUE"
}
}
return instance
} catch (e: Exception) {
val stack = KoinPlatformTools.getStackTrace(e)
context.logger.error("* Instance creation error : could not create instance for '$beanDefinition': $stack")
throw InstanceCreationException("Could not create instance for '$beanDefinition'", e)
}
}

override fun compareTo(other: InstanceFactory<*>): Int {
return instanceCreationOrderPosition.compareTo(other.instanceCreationOrderPosition)
}

/**
* Is instance created
*/
Expand All @@ -72,5 +97,6 @@ abstract class InstanceFactory<T>(val beanDefinition: BeanDefinition<T>) : Locka

companion object {
const val ERROR_SEPARATOR = "\n\t"
private val INSTANCE_CREATION_ORDER_COUNTER = AtomicLong(-1)
}
}
25 changes: 15 additions & 10 deletions projects/core/koin-core/src/commonMain/kotlin/org/koin/core/registry/InstanceRegistry.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ import org.koin.core.scope.Scope
import org.koin.core.scope.ScopeID
import org.koin.mp.KoinPlatformTools.safeHashMap
import kotlin.reflect.KClass
import kotlin.sequences.sortedDescending


@Suppress("UNCHECKED_CAST")
@OptIn(KoinInternalApi::class)
Expand Down Expand Up @@ -152,13 +154,18 @@ class InstanceRegistry(val _koin: Koin) {
}

internal fun dropScopeInstances(scope: Scope) {
_instances.values.filterIsInstance<ScopedInstanceFactory<*>>().forEach { factory -> factory.drop(scope) }
_instances.values
.asSequence()
.filterIsInstance<ScopedInstanceFactory<*>>()
.sortedDescending()
.forEach { factory -> factory.drop(scope) }
}

internal fun close() {
_instances.forEach { (_, factory) ->
factory.dropAll()
}
_instances.values
.asSequence()
.sortedDescending()
.forEach { factory -> factory.dropAll() }
_instances.clear()
}

Expand All @@ -181,12 +188,10 @@ class InstanceRegistry(val _koin: Koin) {
}

private fun unloadModule(module: Module) {
module.mappings.keys.forEach { mapping ->
if (_instances.containsKey(mapping)) {
_instances[mapping]?.dropAll()
_instances.remove(mapping)
}
}
module.mappings.keys
.mapNotNull { mapping -> _instances.remove(mapping) }
.sortedDescending()
.forEach { factory -> factory.dropAll() }
}

fun size(): Int {
Expand Down
186 changes: 186 additions & 0 deletions projects/core/koin-core/src/commonTest/kotlin/org/koin/dsl/CloseDefinitionTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
package org.koin.dsl

import org.koin.Simple
import org.koin.core.annotation.KoinInternalApi
import org.koin.core.logger.Level
import org.koin.core.qualifier.named
import kotlin.test.Test
import kotlin.test.assertEquals
import kotlin.test.assertNotNull
import kotlin.test.assertTrue

class CloseDefinitionTest {
Expand Down Expand Up @@ -50,4 +54,186 @@ class CloseDefinitionTest {
koin.unloadModules(listOf(module))
assertTrue(!closed)
}

@OptIn(KoinInternalApi::class)
@Test
fun test_onClose_order() {
val closeLog = mutableListOf<String>()

val module = module {
single { Simple.ComponentC(get()) } onClose {
if (it != null) {
closeLog += "c"
}
}
single { Simple.ComponentA() } onClose {
if (it != null) {
closeLog += "a"
}
}
single { Simple.ComponentB(get()) } onClose {
if (it != null) {
closeLog += "b"
}
}
}

val koin = koinApplication {
printLogger(Level.DEBUG)
modules(
module,
)
}.koin

assertNotNull(koin.getOrNull<Simple.ComponentC>())
assertEquals(
expected = emptyList(),
actual = closeLog,
)

// check instances sorting meets instantiation order
assertEquals(
expected = listOf(Simple.ComponentA::class, Simple.ComponentB::class, Simple.ComponentC::class),
actual = koin.instanceRegistry.instances.values.sorted().map { it.beanDefinition.primaryType },
)

koin.close()
assertEquals(
expected = listOf("c", "b", "a"),
actual = closeLog,
)
}

@Test
fun test_onClose_order_unload() {
val closeLog = mutableListOf<String>()

val moduleFirst = module {
single { Simple.MySingle(42) } onClose {
if (it != null) {
closeLog += "single"
}
}
}

val moduleSecond = module {
single { Simple.ComponentC(get()) } onClose {
if (it != null) {
closeLog += "c"
}
}
single { Simple.ComponentA() } onClose {
if (it != null) {
closeLog += "a"
}
}
single { Simple.ComponentB(get()) } onClose {
if (it != null) {
closeLog += "b"
}
}
}

val koin = koinApplication {
printLogger(Level.DEBUG)
modules(
moduleFirst,
moduleSecond,
)
}.koin

assertNotNull(koin.getOrNull<Simple.MySingle>())
assertNotNull(koin.getOrNull<Simple.ComponentC>())
assertEquals(emptyList(), closeLog)

koin.unloadModules(listOf(moduleSecond))
assertEquals(
expected = listOf("c", "b", "a"),
actual = closeLog,
)

koin.loadModules(listOf(moduleSecond))
assertNotNull(koin.getOrNull<Simple.ComponentC>())

koin.unloadModules(listOf(moduleSecond))
assertEquals(
expected = listOf("c", "b", "a", "c", "b", "a"),
actual = closeLog,
)

koin.close()
assertEquals(
expected = listOf("c", "b", "a", "c", "b", "a", "single"),
actual = closeLog,
)
}


@Test
fun test_onClose_order_scoped() {
val closeLog = mutableListOf<String>()

val scopeId = "_test_scope_id_"
val scopeKey = named("_test_scope_")

val module = module {
scope(scopeKey) {
scoped { Simple.ComponentC(get()) } onClose {
if (it != null) {
closeLog += "c"
}
}
scoped { Simple.ComponentA() } onClose {
if (it != null) {
closeLog += "a"
}
}
scoped { Simple.ComponentB(get()) } onClose {
if (it != null) {
closeLog += "b"
}
}
}
}

val koin = koinApplication {
printLogger(Level.DEBUG)
modules(
module,
)
}.koin

assertEquals(
expected = emptyList(),
actual = closeLog,
)

koin.createScope(scopeId, scopeKey)
assertEquals(
expected = emptyList(),
actual = closeLog,
)

koin.getScope(scopeId).let {
assertNotNull(it.getOrNull<Simple.ComponentC>())
it.close()
}
assertEquals(
expected = listOf("c", "b", "a"),
actual = closeLog,
)

koin.createScope(scopeId, scopeKey)
assertNotNull(koin.getScope(scopeId).getOrNull<Simple.ComponentC>())
assertEquals(
expected = listOf("c", "b", "a"),
actual = closeLog,
)

koin.close()
assertEquals(
expected = listOf("c", "b", "a", "c", "b", "a"),
actual = closeLog,
)
}
}