Tests that we don't leak EventListeners (#2053)

Also add a new function, TreehouseApp.close()

redwood-treehouse-host/api/android/redwood-treehouse-host.api

@@ -81,6 +81,7 @@ public abstract interface class app/cash/redwood/treehouse/StateStore {
 
 public final class app/cash/redwood/treehouse/TreehouseApp {
 	public synthetic fun <init> (Lapp/cash/redwood/treehouse/TreehouseApp$Factory;Lkotlinx/coroutines/CoroutineScope;Lapp/cash/redwood/treehouse/TreehouseApp$Spec;Lapp/cash/redwood/treehouse/EventListener$Factory;Lkotlin/jvm/internal/DefaultConstructorMarker;)V
+	public final fun close ()V
 	public final fun createContent (Lapp/cash/redwood/treehouse/TreehouseContentSource;Lapp/cash/redwood/treehouse/CodeListener;)Lapp/cash/redwood/treehouse/Content;
 	public static synthetic fun createContent$default (Lapp/cash/redwood/treehouse/TreehouseApp;Lapp/cash/redwood/treehouse/TreehouseContentSource;Lapp/cash/redwood/treehouse/CodeListener;ILjava/lang/Object;)Lapp/cash/redwood/treehouse/Content;
 	public final fun getDispatchers ()Lapp/cash/redwood/treehouse/TreehouseDispatchers;

redwood-treehouse-host/api/jvm/redwood-treehouse-host.api

@@ -81,6 +81,7 @@ public abstract interface class app/cash/redwood/treehouse/StateStore {
 
 public final class app/cash/redwood/treehouse/TreehouseApp {
 	public synthetic fun <init> (Lapp/cash/redwood/treehouse/TreehouseApp$Factory;Lkotlinx/coroutines/CoroutineScope;Lapp/cash/redwood/treehouse/TreehouseApp$Spec;Lapp/cash/redwood/treehouse/EventListener$Factory;Lkotlin/jvm/internal/DefaultConstructorMarker;)V
+	public final fun close ()V
 	public final fun createContent (Lapp/cash/redwood/treehouse/TreehouseContentSource;Lapp/cash/redwood/treehouse/CodeListener;)Lapp/cash/redwood/treehouse/Content;
 	public static synthetic fun createContent$default (Lapp/cash/redwood/treehouse/TreehouseApp;Lapp/cash/redwood/treehouse/TreehouseContentSource;Lapp/cash/redwood/treehouse/CodeListener;ILjava/lang/Object;)Lapp/cash/redwood/treehouse/Content;
 	public final fun getDispatchers ()Lapp/cash/redwood/treehouse/TreehouseDispatchers;

redwood-treehouse-host/api/redwood-treehouse-host.klib.api

@@ -74,6 +74,7 @@ final class <#A: app.cash.redwood.treehouse/AppService> app.cash.redwood.treehou
         final val dispatchers // app.cash.redwood.treehouse/TreehouseApp.Factory.dispatchers|<get-dispatchers>(){}[0]
             final fun <get-dispatchers>(): app.cash.redwood.treehouse/TreehouseDispatchers // app.cash.redwood.treehouse/TreehouseApp.Factory.dispatchers.<get-dispatchers>|<get-dispatchers>(){}[0]
     }
+    final fun close() // app.cash.redwood.treehouse/TreehouseApp.close|close(){}[0]
     final fun createContent(app.cash.redwood.treehouse/TreehouseContentSource<#A>, app.cash.redwood.treehouse/CodeListener =...): app.cash.redwood.treehouse/Content // app.cash.redwood.treehouse/TreehouseApp.createContent|createContent(app.cash.redwood.treehouse.TreehouseContentSource<1:0>;app.cash.redwood.treehouse.CodeListener){}[0]
     final fun restart() // app.cash.redwood.treehouse/TreehouseApp.restart|restart(){}[0]
     final fun start() // app.cash.redwood.treehouse/TreehouseApp.start|start(){}[0]

redwood-treehouse-host/src/appsJvmTest/kotlin/app/cash/redwood/treehouse/LeaksTest.kt

@@ -19,6 +19,7 @@ import app.cash.redwood.treehouse.leaks.LeakWatcher
 import assertk.assertThat
 import assertk.assertions.isEmpty
 import assertk.assertions.isEqualTo
+import assertk.assertions.isNotNull
 import com.example.redwood.testapp.testing.TextInputValue
 import kotlin.test.Test
 import kotlinx.coroutines.flow.first
@@ -34,7 +35,7 @@ class LeaksTest {
 
     content.bind(view)
 
-    content.awaitContent(1)
+    content.awaitContent(untilChangeCount = 1)
     val textInputValue = view.views.single() as TextInputValue
     assertThat(textInputValue.text).isEqualTo("what would you like to see?")
 
@@ -48,7 +49,7 @@ class LeaksTest {
     textInputValue.onChange!!.invoke("Empty")
 
     tester.sendFrame()
-    content.awaitContent(2)
+    content.awaitContent(untilChangeCount = 2)
     assertThat(view.views).isEmpty()
 
     // Once the widget is removed, the cycle must be broken and the widget must be unreachable.
@@ -82,4 +83,42 @@ class LeaksTest {
     tester.eventLog.takeEvent("test_app.codeUnloaded()", skipOthers = true)
     serviceLeakWatcher.assertNotLeaked()
   }
+
+  @Test
+  fun eventListenerNotLeaked() = runTest {
+    val tester = TreehouseTester(this)
+    tester.eventListenerFactory = RetainEverythingEventListenerFactory(tester.eventLog)
+    val treehouseApp = tester.loadApp()
+    val content = tester.content(treehouseApp)
+    val view = tester.view()
+
+    content.bind(view)
+    content.awaitContent(untilChangeCount = 1)
+
+    val eventListenerLeakWatcher = LeakWatcher {
+      (tester.eventListenerFactory as RetainEverythingEventListenerFactory)
+        .also {
+          assertThat(it.app).isNotNull()
+          assertThat(it.manifestUrl).isNotNull()
+          assertThat(it.zipline).isNotNull()
+          assertThat(it.ziplineManifest).isNotNull()
+        }
+    }
+
+    // Stop referencing our EventListener from our test harness.
+    tester.eventListenerFactory = FakeEventListener.Factory(tester.eventLog)
+
+    // While the listener is in a running app, it's expected to be in a reference cycle.
+    eventListenerLeakWatcher.assertObjectInReferenceCycle()
+
+    // It's still in a reference cycle after 'stop', because it can be started again.
+    treehouseApp.stop()
+    treehouseApp.zipline.first { it == null }
+    tester.eventLog.takeEvent("codeUnloaded", skipOthers = true)
+    eventListenerLeakWatcher.assertObjectInReferenceCycle()
+
+    // But after close, it's unreachable.
+    treehouseApp.close()
+    eventListenerLeakWatcher.assertNotLeaked()
+  }
 }

redwood-treehouse-host/src/appsJvmTest/kotlin/app/cash/redwood/treehouse/RetainEverythingEventListenerFactory.kt

@@ -0,0 +1,44 @@
+/*
+ * Copyright (C) 2024 Square, Inc.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package app.cash.redwood.treehouse
+
+import app.cash.zipline.Zipline
+import app.cash.zipline.ZiplineManifest
+
+/** An event listener that keeps a reference to everything it sees, for defensive leak testing. */
+class RetainEverythingEventListenerFactory(
+  private val eventLog: EventLog,
+) : EventListener(), EventListener.Factory {
+  var app: TreehouseApp<*>? = null
+  var manifestUrl: String? = null
+  var zipline: Zipline? = null
+  var ziplineManifest: ZiplineManifest? = null
+
+  override fun create(app: TreehouseApp<*>, manifestUrl: String?): EventListener {
+    this.app = app
+    this.manifestUrl = manifestUrl
+    return this
+  }
+
+  override fun codeLoadSuccess(manifest: ZiplineManifest, zipline: Zipline, startValue: Any?) {
+    this.zipline = zipline
+    this.ziplineManifest = manifest
+  }
+
+  override fun codeUnloaded() {
+    eventLog += "codeUnloaded"
+  }
+}

redwood-treehouse-host/src/appsTest/kotlin/app/cash/redwood/treehouse/TreehouseTester.kt

@@ -45,6 +45,10 @@ internal class TreehouseTester(
 ) {
   val eventLog = EventLog()
 
+  var hostApi: HostApi = FakeHostApi()
+
+  var eventListenerFactory: EventListener.Factory = FakeEventListener.Factory(eventLog)
+
   @OptIn(ExperimentalStdlibApi::class)
   private val testDispatcher = testScope.coroutineContext[CoroutineDispatcher.Key] as TestDispatcher
 
@@ -96,8 +100,6 @@ internal class TreehouseTester(
     override fun create(scope: CoroutineScope, dispatchers: TreehouseDispatchers) = frameClock
   }
 
-  var hostApi: HostApi = FakeHostApi()
-
   private val treehouseAppFactory = TreehouseApp.Factory(
     platform = platform,
     dispatchers = dispatchers,
@@ -133,7 +135,7 @@ internal class TreehouseTester(
     return treehouseAppFactory.create(
       appScope = testScope,
       spec = appSpec,
-      eventListenerFactory = FakeEventListener.Factory(eventLog),
+      eventListenerFactory = eventListenerFactory,
     )
   }
 

redwood-treehouse-host/src/commonMain/kotlin/app/cash/redwood/treehouse/CodeHost.kt

@@ -87,9 +87,11 @@ internal abstract class CodeHost<A : AppService>(
     get() = state.codeSession
 
   /** Returns a flow that emits a new [CodeSession] each time we should load fresh code. */
-  abstract fun codeUpdatesFlow(): Flow<CodeSession<A>>
+  abstract fun codeUpdatesFlow(
+    eventListenerFactory: EventListener.Factory,
+  ): Flow<CodeSession<A>>
 
-  fun start() {
+  fun start(eventListenerFactory: EventListener.Factory) {
     dispatchers.checkUi()
 
     val previous = state
@@ -101,7 +103,7 @@ internal abstract class CodeHost<A : AppService>(
 
     val codeUpdatesScope = newCodeUpdatesScope()
     state = State.Starting(codeUpdatesScope)
-    codeUpdatesScope.collectCodeUpdates()
+    codeUpdatesScope.collectCodeUpdates(eventListenerFactory)
   }
 
   /** This function may only be invoked on [TreehouseDispatchers.zipline]. */
@@ -117,7 +119,7 @@ internal abstract class CodeHost<A : AppService>(
     mutableZipline.value = null
   }
 
-  fun restart() {
+  fun restart(eventListenerFactory: EventListener.Factory) {
     dispatchers.checkUi()
 
     val previous = state
@@ -130,7 +132,7 @@ internal abstract class CodeHost<A : AppService>(
     val codeUpdatesScope = newCodeUpdatesScope()
     state = State.Starting(codeUpdatesScope)
     mutableZipline.value = null
-    codeUpdatesScope.collectCodeUpdates()
+    codeUpdatesScope.collectCodeUpdates(eventListenerFactory)
   }
 
   fun addListener(listener: Listener<A>) {
@@ -146,9 +148,9 @@ internal abstract class CodeHost<A : AppService>(
   private fun newCodeUpdatesScope() =
     CoroutineScope(SupervisorJob(appScope.coroutineContext.job))
 
-  private fun CoroutineScope.collectCodeUpdates() {
+  private fun CoroutineScope.collectCodeUpdates(eventListenerFactory: EventListener.Factory) {
     launch(dispatchers.zipline) {
-      codeUpdatesFlow().collect {
+      codeUpdatesFlow(eventListenerFactory).collect {
         codeSessionLoaded(it)
       }
     }

redwood-treehouse-host/src/commonMain/kotlin/app/cash/redwood/treehouse/CodeSession.kt

@@ -83,6 +83,7 @@ internal abstract class CodeSession<A : AppService>(
     scope.launch(dispatchers.zipline, start = CoroutineStart.ATOMIC) {
       ziplineStop()
       scope.cancel()
+      eventPublisher.close() // Must be last to prevent lost events.
     }
   }
 

redwood-treehouse-host/src/commonMain/kotlin/app/cash/redwood/treehouse/EventPublisher.kt

@@ -31,4 +31,6 @@ internal interface EventPublisher {
   fun onUnknownEventNode(id: Id, tag: EventTag)
 
   fun onUncaughtException(exception: Throwable)
+
+  fun close()
 }