Repository: arkivanov/Decompose
Pull request #238 (2021-10-03 19:13:33)

Fixed a memory leak in MergedLifecycle


Patch:
diff --git a/decompose/src/commonMain/kotlin/com/arkivanov/decompose/lifecycle/MergedLifecycle.kt b/decompose/src/commonMain/kotlin/com/arkivanov/decompose/lifecycle/MergedLifecycle.kt
index 727538c6..3f257e87 100644
--- a/decompose/src/commonMain/kotlin/com/arkivanov/decompose/lifecycle/MergedLifecycle.kt
+++ b/decompose/src/commonMain/kotlin/com/arkivanov/decompose/lifecycle/MergedLifecycle.kt
@@ -3,6 +3,7 @@ package com.arkivanov.decompose.lifecycle
 import com.arkivanov.decompose.InternalDecomposeApi
 import com.arkivanov.essenty.lifecycle.Lifecycle
 import com.arkivanov.essenty.lifecycle.LifecycleRegistry
+import com.arkivanov.essenty.lifecycle.doOnDestroy
 
 @InternalDecomposeApi
 class MergedLifecycle private constructor(
@@ -13,17 +14,30 @@ class MergedLifecycle private constructor(
 
     constructor(lifecycle1: Lifecycle, lifecycle2: Lifecycle) : this(LifecycleRegistry(), lifecycle1, lifecycle2)
 
-    private var state1: Lifecycle.State = Lifecycle.State.INITIALIZED
-    private var state2: Lifecycle.State = Lifecycle.State.INITIALIZED
-
     init {
-        lifecycle1.subscribe(LifecycleObserverImpl(otherState = ::state2, onStateChanged = { state1 = it }))
-        lifecycle2.subscribe(LifecycleObserverImpl(otherState = ::state1, onStateChanged = { state2 = it }))
+        if ((lifecycle1.state == Lifecycle.State.DESTROYED) || (lifecycle2.state == Lifecycle.State.DESTROYED)) {
+            registry.onCreate()
+            registry.onDestroy()
+        } else {
+            var state1: Lifecycle.State = Lifecycle.State.INITIALIZED
+            var state2: Lifecycle.State = Lifecycle.State.INITIALIZED
+            val observer1 = CallbacksImpl(registry = registry, setState = { state1 = it }, getOtherState = { state2 })
+            val observer2 = CallbacksImpl(registry = registry, setState = { state2 = it }, getOtherState = { state1 })
+
+            lifecycle1.subscribe(observer1)
+            lifecycle2.subscribe(observer2)
+
+            registry.doOnDestroy {
+                lifecycle1.unsubscribe(observer1)
+                lifecycle2.unsubscribe(observer2)
+            }
+        }
     }
 
-    private inner class LifecycleObserverImpl(
-        private val otherState: () -> Lifecycle.State,
-        private val onStateChanged: (Lifecycle.State) -> Unit
+    private class CallbacksImpl(
+        private val registry: Lifecycle.Callbacks,
+        private val setState: (Lifecycle.State) -> Unit,
+        private val getOtherState: () -> Lifecycle.State,
     ) : Lifecycle.Callbacks {
         override fun onCreate() {
             onUp(Lifecycle.State.CREATED, registry::onCreate)
@@ -50,17 +64,18 @@ class MergedLifecycle private constructor(
         }
 
         private fun onUp(state: Lifecycle.State, call: () -> Unit) {
-            onStateChanged(state)
-            if (state <= otherState()) {
+            setState(state)
+            if (state <= getOtherState()) {
                 call()
             }
         }
 
         private fun onDown(state: Lifecycle.State, call: () -> Unit) {
-            if (state < otherState()) {
+            if (state < getOtherState()) {
                 call()
             }
-            onStateChanged(state)
+            setState(state)
         }
     }
 }
+


Test patch:
diff --git a/decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/lifecycle/MergedLifecycleTest.kt b/decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/lifecycle/MergedLifecycleTest.kt
new file mode 100644
index 00000000..3eebb11b
--- /dev/null
+++ b/decompose/src/commonTest/kotlin/com/arkivanov/decompose/router/lifecycle/MergedLifecycleTest.kt
@@ -0,0 +1,502 @@
+package com.arkivanov.decompose.router.lifecycle
+
+import com.arkivanov.decompose.lifecycle.MergedLifecycle
+import com.arkivanov.decompose.router.lifecycle.MergedLifecycleTest.TestLifecycleCallbacks.Event
+import com.arkivanov.essenty.lifecycle.Lifecycle
+import com.arkivanov.essenty.lifecycle.LifecycleRegistry
+import com.arkivanov.essenty.lifecycle.create
+import com.arkivanov.essenty.lifecycle.destroy
+import com.arkivanov.essenty.lifecycle.pause
+import com.arkivanov.essenty.lifecycle.resume
+import com.arkivanov.essenty.lifecycle.start
+import com.arkivanov.essenty.lifecycle.stop
+import kotlin.test.Test
+import kotlin.test.assertEquals
+
+@Suppress("TestFunctionName")
+class MergedLifecycleTest {
+
+    private val lifecycle1 = TestLifecycleRegistry()
+    private val lifecycle2 = TestLifecycleRegistry()
+    private val merged = MergedLifecycle(lifecycle1, lifecycle2)
+    private val callbacks = TestLifecycleCallbacks()
+
+    init {
+        merged.subscribe(callbacks)
+    }
+
+    @Test
+    fun GIVEN_lifecycle1_initialized_WHEN_lifecycle2_created_THEN_no_events() {
+        lifecycle2.create()
+
+        callbacks.assertNoEvents()
+    }
+
+    @Test
+    fun GIVEN_lifecycle2_initialized_WHEN_lifecycle1_created_THEN_no_events() {
+        lifecycle1.create()
+
+        callbacks.assertNoEvents()
+    }
+
+    @Test
+    fun GIVEN_lifecycle1_initialized_WHEN_lifecycle2_started_THEN_no_events() {
+        lifecycle2.start()
+
+        callbacks.assertNoEvents()
+    }
+
+    @Test
+    fun GIVEN_lifecycle2_initialized_WHEN_lifecycle1_started_THEN_no_events() {
+        lifecycle1.start()
+
+        callbacks.assertNoEvents()
+    }
+
+    @Test
+    fun GIVEN_lifecycle1_initialized_WHEN_lifecycle2_resumed_THEN_no_events() {
+        lifecycle2.resume()
+
+        callbacks.assertNoEvents()
+    }
+
+    @Test
+    fun GIVEN_lifecycle2_initialized_WHEN_lifecycle1_resumed_THEN_no_events() {
+        lifecycle1.resume()
+
+        callbacks.assertNoEvents()
+    }
+
+    @Test
+    fun GIVEN_lifecycle1_created_WHEN_lifecycle2_created_THEN_onCreate_called() {
+        lifecycle1.create()
+
+        lifecycle2.create()
+
+        callbacks.assertEvents(Event.ON_CREATE)
+    }
+
+    @Test
+    fun GIVEN_lifecycle2_created_WHEN_lifecycle1_created_THEN_onCreate_called() {
+        lifecycle2.create()
+
+        lifecycle1.create()
+
+        callbacks.assertEvents(Event.ON_CREATE)
+    }
+
+    @Test
+    fun GIVEN_lifecycle1_created_WHEN_lifecycle2_started_THEN_onCreate_called() {
+        lifecycle1.create()
+
+        lifecycle2.start()
+
+        callbacks.assertEvents(Event.ON_CREATE)
+    }
+
+    @Test
+    fun GIVEN_lifecycle2_created_WHEN_lifecycle1_started_THEN_onCreate_called() {
+        lifecycle2.create()
+
+        lifecycle1.start()
+
+        callbacks.assertEvents(Event.ON_CREATE)
+    }
+
+    @Test
+    fun GIVEN_lifecycle1_created_WHEN_lifecycle2_resumed_THEN_onCreate_called() {
+        lifecycle1.create()
+
+        lifecycle2.resume()
+
+        callbacks.assertEvents(Event.ON_CREATE)
+    }
+
+    @Test
+    fun GIVEN_lifecycle2_created_WHEN_lifecycle1_resumed_THEN_onCreate_called() {
+        lifecycle2.create()
+
+        lifecycle1.resume()
+
+        callbacks.assertEvents(Event.ON_CREATE)
+    }
+
+    @Test
+    fun GIVEN_lifecycle1_started_WHEN_lifecycle2_created_THEN_onCreate_called() {
+        lifecycle1.start()
+
+        lifecycle2.create()
+
+        callbacks.assertEvents(Event.ON_CREATE)
+    }
+
+    @Test
+    fun GIVEN_lifecycle2_started_WHEN_lifecycle1_created_THEN_onCreate_called() {
+        lifecycle2.start()
+
+        lifecycle1.create()
+
+        callbacks.assertEvents(Event.ON_CREATE)
+    }
+
+    @Test
+    fun GIVEN_lifecycle1_started_WHEN_lifecycle2_started_THEN_onCreate_onStart_called() {
+        lifecycle1.start()
+
+        lifecycle2.start()
+
+        callbacks.assertEvents(Event.ON_CREATE, Event.ON_START)
+    }
+
+    @Test
+    fun GIVEN_lifecycle2_started_WHEN_lifecycle1_started_THEN_onCreate_onStart_called() {
+        lifecycle2.start()
+
+        lifecycle1.start()
+
+        callbacks.assertEvents(Event.ON_CREATE, Event.ON_START)
+    }
+
+    @Test
+    fun GIVEN_lifecycle1_started_WHEN_lifecycle2_resumed_THEN_onCreate_onStart_called() {
+        lifecycle1.start()
+
+        lifecycle2.resume()
+
+        callbacks.assertEvents(Event.ON_CREATE, Event.ON_START)
+    }
+
+    @Test
+    fun GIVEN_lifecycle2_started_WHEN_lifecycle1_resumed_THEN_onCreate_onStart_called() {
+        lifecycle2.start()
+
+        lifecycle1.resume()
+
+        callbacks.assertEvents(Event.ON_CREATE, Event.ON_START)
+    }
+
+
+    @Test
+    fun GIVEN_lifecycle1_resumed_WHEN_lifecycle2_created_THEN_onCreate_called() {
+        lifecycle1.resume()
+
+        lifecycle2.create()
+
+        callbacks.assertEvents(Event.ON_CREATE)
+    }
+
+    @Test
+    fun GIVEN_lifecycle2_resumed_WHEN_lifecycle1_created_THEN_onCreate_called() {
+        lifecycle2.resume()
+
+        lifecycle1.create()
+
+        callbacks.assertEvents(Event.ON_CREATE)
+    }
+
+    @Test
+    fun GIVEN_lifecycle1_resumed_WHEN_lifecycle2_started_THEN_onCreate_onStart_called() {
+        lifecycle1.resume()
+
+        lifecycle2.start()
+
+        callbacks.assertEvents(Event.ON_CREATE, Event.ON_START)
+    }
+
+    @Test
+    fun GIVEN_lifecycle2_resumed_WHEN_lifecycle1_started_THEN_onCreate_onStart_called() {
+        lifecycle2.resume()
+
+        lifecycle1.start()
+
+        callbacks.assertEvents(Event.ON_CREATE, Event.ON_START)
+    }
+
+    @Test
+    fun GIVEN_lifecycle1_resumed_WHEN_lifecycle2_resumed_THEN_onCreate_onStart_onResume_called() {
+        lifecycle1.resume()
+
+        lifecycle2.resume()
+
+        callbacks.assertEvents(Event.ON_CREATE, Event.ON_START, Event.ON_RESUME)
+    }
+
+    @Test
+    fun GIVEN_lifecycle2_resumed_WHEN_lifecycle1_resumed_THEN_onCreate_onStart_called() {
+        lifecycle2.resume()
+
+        lifecycle1.resume()
+
+        callbacks.assertEvents(Event.ON_CREATE, Event.ON_START, Event.ON_RESUME)
+    }
+
+    @Test
+    fun GIVEN_lifecycle1_resumed_and_lifecycle2_resumed_WHEN_lifecycle2_paused_THEN_onPause_called() {
+        lifecycle1.resume()
+        lifecycle2.resume()
+        callbacks.clear()
+
+        lifecycle2.pause()
+
+        callbacks.assertEvents(Event.ON_PAUSE)
+    }
+
+    @Test
+    fun GIVEN_lifecycle1_resumed_and_lifecycle2_resumed_WHEN_lifecycle1_paused_THEN_onPause_called() {
+        lifecycle1.resume()
+        lifecycle2.resume()
+        callbacks.clear()
+
+        lifecycle1.pause()
+
+        callbacks.assertEvents(Event.ON_PAUSE)
+    }
+
+    @Test
+    fun GIVEN_lifecycle1_resumed_and_lifecycle2_resumed_WHEN_lifecycle2_stopped_THEN_onPause_onStop_called() {
+        lifecycle1.resume()
+        lifecycle2.resume()
+        callbacks.clear()
+
+        lifecycle2.stop()
+
+        callbacks.assertEvents(Event.ON_PAUSE, Event.ON_STOP)
+    }
+
+    @Test
+    fun GIVEN_lifecycle1_resumed_and_lifecycle2_resumed_WHEN_lifecycle1_stopped_THEN_onPause_called() {
+        lifecycle1.resume()
+        lifecycle2.resume()
+        callbacks.clear()
+
+        lifecycle1.stop()
+
+        callbacks.assertEvents(Event.ON_PAUSE, Event.ON_STOP)
+    }
+
+    @Test
+    fun GIVEN_lifecycle1_resumed_and_lifecycle2_resumed_WHEN_lifecycle2_destroyed_THEN_onPause_onStop_onDestroy_called() {
+        lifecycle1.resume()
+        lifecycle2.resume()
+        callbacks.clear()
+
+        lifecycle2.destroy()
+
+        callbacks.assertEvents(Event.ON_PAUSE, Event.ON_STOP, Event.ON_DESTROY)
+    }
+
+    @Test
+    fun GIVEN_lifecycle1_resumed_and_lifecycle2_resumed_WHEN_lifecycle1_destroyed_THEN_onPause_onStop_onDestroy_called() {
+        lifecycle1.resume()
+        lifecycle2.resume()
+        callbacks.clear()
+
+        lifecycle1.destroy()
+
+        callbacks.assertEvents(Event.ON_PAUSE, Event.ON_STOP, Event.ON_DESTROY)
+    }
+
+    @Test
+    fun GIVEN_lifecycle1_paused_and_lifecycle2_resumed_WHEN_lifecycle2_paused_THEN_no_events() {
+        lifecycle1.resume()
+        lifecycle2.resume()
+        lifecycle1.pause()
+        callbacks.clear()
+
+        lifecycle2.pause()
+
+        callbacks.assertNoEvents()
+    }
+
+    @Test
+    fun GIVEN_lifecycle1_resumed_and_lifecycle2_paused_WHEN_lifecycle1_paused_THEN_no_events() {
+        lifecycle1.resume()
+        lifecycle2.resume()
+        lifecycle2.pause()
+        callbacks.clear()
+
+        lifecycle1.pause()
+
+        callbacks.assertNoEvents()
+    }
+
+    @Test
+    fun GIVEN_lifecycle1_stopped_and_lifecycle2_resumed_WHEN_lifecycle2_stopped_THEN_no_events() {
+        lifecycle1.resume()
+        lifecycle2.resume()
+        lifecycle1.stop()
+        callbacks.clear()
+
+        lifecycle2.stop()
+
+        callbacks.assertNoEvents()
+    }
+
+    @Test
+    fun GIVEN_lifecycle1_resumed_and_lifecycle2_stopped_WHEN_lifecycle1_stopped_THEN_no_events() {
+        lifecycle1.resume()
+        lifecycle2.resume()
+        lifecycle2.stop()
+        callbacks.clear()
+
+        lifecycle1.stop()
+
+        callbacks.assertNoEvents()
+    }
+
+    @Test
+    fun GIVEN_lifecycle1_destroyed_and_lifecycle2_resumed_WHEN_lifecycle2_destroyed_THEN_no_events() {
+        lifecycle1.resume()
+        lifecycle2.resume()
+        lifecycle1.destroy()
+        callbacks.clear()
+
+        lifecycle2.destroy()
+
+        callbacks.assertNoEvents()
+    }
+
+    @Test
+    fun GIVEN_lifecycle1_resumed_and_lifecycle2_destroyed_WHEN_lifecycle1_destroyed_THEN_no_events() {
+        lifecycle1.resume()
+        lifecycle2.resume()
+        lifecycle2.destroy()
+        callbacks.clear()
+
+        lifecycle1.destroy()
+
+        callbacks.assertNoEvents()
+    }
+
+    @Test
+    fun GIVEN_lifecycle1_destroyed_WHEN_MergedLifecycle_created_THEN_not_subscribed() {
+        val lifecycle1 = TestLifecycleRegistry()
+        val lifecycle2 = TestLifecycleRegistry()
+        lifecycle1.create()
+        lifecycle1.destroy()
+
+        MergedLifecycle(lifecycle1, lifecycle2)
+
+        lifecycle1.assertNoSubscribers()
+        lifecycle2.assertNoSubscribers()
+    }
+
+    @Test
+    fun GIVEN_lifecycle2_destroyed_WHEN_MergedLifecycle_created_THEN_not_subscribed() {
+        val lifecycle1 = TestLifecycleRegistry()
+        val lifecycle2 = TestLifecycleRegistry()
+        lifecycle2.create()
+        lifecycle2.destroy()
+
+        MergedLifecycle(lifecycle1, lifecycle2)
+
+        lifecycle1.assertNoSubscribers()
+        lifecycle2.assertNoSubscribers()
+    }
+
+    @Test
+    fun GIVEN_lifecycle1_destroyed_WHEN_MergedLifecycle_created_THEN_state_destroyed() {
+        lifecycle1.create()
+        lifecycle1.destroy()
+
+        val merged = MergedLifecycle(lifecycle1, lifecycle2)
+
+        assertEquals(Lifecycle.State.DESTROYED, merged.state)
+    }
+
+    @Test
+    fun GIVEN_lifecycle2_destroyed_WHEN_MergedLifecycle_created_THEN_state_destroyed() {
+        lifecycle2.create()
+        lifecycle2.destroy()
+
+        val merged = MergedLifecycle(lifecycle1, lifecycle2)
+
+        assertEquals(Lifecycle.State.DESTROYED, merged.state)
+    }
+
+    @Test
+    fun WHEN_lifecycle1_destroyed_THEN_all_callbacks_unsubscribed() {
+        lifecycle1.resume()
+        lifecycle2.resume()
+
+        lifecycle1.destroy()
+
+        lifecycle1.assertNoSubscribers()
+        lifecycle2.assertNoSubscribers()
+    }
+
+    @Test
+    fun WHEN_lifecycle2_destroyed_THEN_all_callbacks_unsubscribed() {
+        lifecycle1.resume()
+        lifecycle2.resume()
+
+        lifecycle2.destroy()
+
+        lifecycle1.assertNoSubscribers()
+        lifecycle2.assertNoSubscribers()
+    }
+
+    private class TestLifecycleRegistry(
+        private val registry: LifecycleRegistry = LifecycleRegistry()
+    ): LifecycleRegistry by registry {
+        private val callbacks = HashSet<Lifecycle.Callbacks>()
+
+        override fun subscribe(callbacks: Lifecycle.Callbacks) {
+            this.callbacks += callbacks
+            registry.subscribe(callbacks)
+        }
+
+        override fun unsubscribe(callbacks: Lifecycle.Callbacks) {
+            this.callbacks -= callbacks
+            registry.unsubscribe(callbacks)
+        }
+
+        fun assertNoSubscribers() {
+            assertEquals(emptySet(), callbacks)
+        }
+    }
+
+    private class TestLifecycleCallbacks : Lifecycle.Callbacks {
+        private val events = ArrayList<Event>()
+
+        override fun onCreate() {
+            events += Event.ON_CREATE
+        }
+
+        override fun onStart() {
+            events += Event.ON_START
+        }
+
+        override fun onResume() {
+            events += Event.ON_RESUME
+        }
+
+        override fun onPause() {
+            events += Event.ON_PAUSE
+        }
+
+        override fun onStop() {
+            events += Event.ON_STOP
+        }
+
+        override fun onDestroy() {
+            events += Event.ON_DESTROY
+        }
+
+        fun assertEvents(vararg events: Event) {
+            assertEquals(events.toList(), this.events)
+        }
+
+        fun assertNoEvents() {
+            assertEquals(emptyList(), events)
+        }
+
+        fun clear() {
+            events.clear()
+        }
+
+        enum class Event {
+            ON_CREATE, ON_START, ON_RESUME, ON_PAUSE, ON_STOP, ON_DESTROY
+        }
+    }
+}