Detect host-side node, widget, and view leaks

redwood-compose/src/commonTest/kotlin/app/cash/redwood/compose/ChangeListenerTest.kt

@@ -22,6 +22,7 @@ import app.cash.redwood.Modifier
 import app.cash.redwood.RedwoodCodegenApi
 import app.cash.redwood.layout.testing.RedwoodLayoutTestingWidgetFactory
 import app.cash.redwood.lazylayout.testing.RedwoodLazyLayoutTestingWidgetFactory
+import app.cash.redwood.leaks.LeakDetector
 import app.cash.redwood.protocol.guest.DefaultGuestProtocolAdapter
 import app.cash.redwood.protocol.guest.guestRedwoodVersion
 import app.cash.redwood.protocol.host.HostProtocolAdapter
@@ -67,6 +68,7 @@ class ProtocolChangeListenerTest : AbstractChangeListenerTest() {
       container = MutableListChildren(),
       factory = TestSchemaProtocolFactory(widgetSystem),
       eventSink = { throw AssertionError() },
+      leakDetector = LeakDetector.None,
     )
     guestAdapter.initChangesSink(hostAdapter)
     return TestRedwoodComposition(this, guestAdapter.widgetSystem, guestAdapter.root) {

redwood-gradle-plugin/build.gradle

@@ -113,6 +113,7 @@ gradlePlugin {
 test {
   dependsOn(':redwood-compose:publishAllPublicationsToLocalMavenRepository')
   dependsOn(':redwood-gradle-plugin:publishAllPublicationsToLocalMavenRepository')
+  dependsOn(':redwood-leak-detector:publishAllPublicationsToLocalMavenRepository')
   dependsOn(':redwood-protocol:publishAllPublicationsToLocalMavenRepository')
   dependsOn(':redwood-protocol-guest:publishAllPublicationsToLocalMavenRepository')
   dependsOn(':redwood-protocol-host:publishAllPublicationsToLocalMavenRepository')

redwood-leak-detector-zipline-test/src/guestMain/kotlin/app/cash/redwood/leaks/zipline/LeakDetectorTestServiceImpl.kt

@@ -21,19 +21,15 @@ import app.cash.redwood.leaks.zipline.LeakDetectorTestService.Companion.SERVICE_
 import app.cash.zipline.Zipline
 import assertk.assertThat
 import assertk.assertions.isSameInstanceAs
-import kotlin.time.Duration
 import kotlin.time.Duration.Companion.seconds
 import kotlin.time.TimeSource
 
 class LeakDetectorTestServiceImpl : LeakDetectorTestService {
   override fun leakDetectorDisabled() {
     val leakDetector = LeakDetector.timeBased(
-      listener = object : LeakListener {
-        override fun onReferenceCollected(name: String) {}
-        override fun onReferenceLeaked(name: String, alive: Duration) {}
-      },
       timeSource = TimeSource.Monotonic,
       leakThreshold = 2.seconds,
+      listener = object : LeakListener() {},
     )
     // QuickJS does not support WeakRef which is required for the leak detection to work correctly.
     // Once WeakRef is supported and this test starts failing, enable bridging of the real tests.

redwood-leak-detector/api/redwood-leak-detector.api

@@ -1,22 +1,23 @@
 public abstract interface class app/cash/redwood/leaks/LeakDetector {
 	public static final field Companion Lapp/cash/redwood/leaks/LeakDetector$Companion;
 	public abstract fun checkLeaks (Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
-	public abstract fun watchReference (Ljava/lang/Object;Ljava/lang/String;)V
+	public abstract fun watchReference (Ljava/lang/Object;Lkotlin/jvm/functions/Function0;)V
 }
 
 public final class app/cash/redwood/leaks/LeakDetector$Companion {
-	public final fun timeBased-SxA4cEA (Lapp/cash/redwood/leaks/LeakListener;Lkotlin/time/TimeSource;J)Lapp/cash/redwood/leaks/LeakDetector;
+	public final fun timeBased-8Mi8wO0 (Lkotlin/time/TimeSource;JLapp/cash/redwood/leaks/LeakListener;)Lapp/cash/redwood/leaks/LeakDetector;
 }
 
 public final class app/cash/redwood/leaks/LeakDetector$None : app/cash/redwood/leaks/LeakDetector {
 	public static final field INSTANCE Lapp/cash/redwood/leaks/LeakDetector$None;
 	public fun checkLeaks (Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
-	public fun watchReference (Ljava/lang/Object;Ljava/lang/String;)V
+	public fun watchReference (Ljava/lang/Object;Lkotlin/jvm/functions/Function0;)V
 }
 
-public abstract interface class app/cash/redwood/leaks/LeakListener {
-	public abstract fun onReferenceCollected (Ljava/lang/String;)V
-	public abstract fun onReferenceLeaked-HG0u8IE (Ljava/lang/String;J)V
+public abstract class app/cash/redwood/leaks/LeakListener {
+	public fun <init> ()V
+	public fun onReferenceCollected (Lkotlin/jvm/functions/Function0;)V
+	public fun onReferenceLeaked-8Mi8wO0 (Ljava/lang/Object;JLkotlin/jvm/functions/Function0;)V
 }
 
 public abstract interface annotation class app/cash/redwood/leaks/RedwoodLeakApi : java/lang/annotation/Annotation {

redwood-leak-detector/api/redwood-leak-detector.klib.api

@@ -11,20 +11,22 @@ open annotation class app.cash.redwood.leaks/RedwoodLeakApi : kotlin/Annotation
 }
 
 abstract interface app.cash.redwood.leaks/LeakDetector { // app.cash.redwood.leaks/LeakDetector|null[0]
-    abstract fun watchReference(kotlin/Any, kotlin/String) // app.cash.redwood.leaks/LeakDetector.watchReference|watchReference(kotlin.Any;kotlin.String){}[0]
+    abstract fun watchReference(kotlin/Any, kotlin/Function0<kotlin/String>) // app.cash.redwood.leaks/LeakDetector.watchReference|watchReference(kotlin.Any;kotlin.Function0<kotlin.String>){}[0]
     abstract suspend fun checkLeaks() // app.cash.redwood.leaks/LeakDetector.checkLeaks|checkLeaks(){}[0]
 
     final object Companion { // app.cash.redwood.leaks/LeakDetector.Companion|null[0]
-        final fun timeBased(app.cash.redwood.leaks/LeakListener, kotlin.time/TimeSource, kotlin.time/Duration): app.cash.redwood.leaks/LeakDetector // app.cash.redwood.leaks/LeakDetector.Companion.timeBased|timeBased(app.cash.redwood.leaks.LeakListener;kotlin.time.TimeSource;kotlin.time.Duration){}[0]
+        final fun timeBased(kotlin.time/TimeSource, kotlin.time/Duration, app.cash.redwood.leaks/LeakListener): app.cash.redwood.leaks/LeakDetector // app.cash.redwood.leaks/LeakDetector.Companion.timeBased|timeBased(kotlin.time.TimeSource;kotlin.time.Duration;app.cash.redwood.leaks.LeakListener){}[0]
     }
 
     final object None : app.cash.redwood.leaks/LeakDetector { // app.cash.redwood.leaks/LeakDetector.None|null[0]
-        final fun watchReference(kotlin/Any, kotlin/String) // app.cash.redwood.leaks/LeakDetector.None.watchReference|watchReference(kotlin.Any;kotlin.String){}[0]
+        final fun watchReference(kotlin/Any, kotlin/Function0<kotlin/String>) // app.cash.redwood.leaks/LeakDetector.None.watchReference|watchReference(kotlin.Any;kotlin.Function0<kotlin.String>){}[0]
         final suspend fun checkLeaks() // app.cash.redwood.leaks/LeakDetector.None.checkLeaks|checkLeaks(){}[0]
     }
 }
 
-abstract interface app.cash.redwood.leaks/LeakListener { // app.cash.redwood.leaks/LeakListener|null[0]
-    abstract fun onReferenceCollected(kotlin/String) // app.cash.redwood.leaks/LeakListener.onReferenceCollected|onReferenceCollected(kotlin.String){}[0]
-    abstract fun onReferenceLeaked(kotlin/String, kotlin.time/Duration) // app.cash.redwood.leaks/LeakListener.onReferenceLeaked|onReferenceLeaked(kotlin.String;kotlin.time.Duration){}[0]
+abstract class app.cash.redwood.leaks/LeakListener { // app.cash.redwood.leaks/LeakListener|null[0]
+    constructor <init>() // app.cash.redwood.leaks/LeakListener.<init>|<init>(){}[0]
+
+    open fun onReferenceCollected(kotlin/Function0<kotlin/String>) // app.cash.redwood.leaks/LeakListener.onReferenceCollected|onReferenceCollected(kotlin.Function0<kotlin.String>){}[0]
+    open fun onReferenceLeaked(kotlin/Any, kotlin.time/Duration, kotlin/Function0<kotlin/String>) // app.cash.redwood.leaks/LeakListener.onReferenceLeaked|onReferenceLeaked(kotlin.Any;kotlin.time.Duration;kotlin.Function0<kotlin.String>){}[0]
 }

redwood-leak-detector/src/commonMain/kotlin/app/cash/redwood/leaks/LeakDetector.kt

@@ -27,7 +27,7 @@ public interface LeakDetector {
    *
    * This function is safe to call from any thread.
    */
-  public fun watchReference(reference: Any, name: String)
+  public fun watchReference(reference: Any, description: () -> String)
 
   /**
    * Trigger garbage collection and determine if any watched references have leaked per this
@@ -38,7 +38,7 @@ public interface LeakDetector {
   public suspend fun checkLeaks()
 
   public object None : LeakDetector {
-    override fun watchReference(reference: Any, name: String) {}
+    override fun watchReference(reference: Any, description: () -> String) {}
     override suspend fun checkLeaks() {}
   }
 
@@ -51,9 +51,9 @@ public interface LeakDetector {
      * case [listener] will never be invoked.
      */
     public fun timeBased(
-      listener: LeakListener,
       timeSource: TimeSource,
       leakThreshold: Duration,
+      listener: LeakListener,
     ): LeakDetector {
       if (hasWeakReference()) {
         return TimeBasedLeakDetector(listener, timeSource, leakThreshold)
@@ -64,9 +64,9 @@ public interface LeakDetector {
 }
 
 @RedwoodLeakApi
-public interface LeakListener {
-  public fun onReferenceCollected(name: String)
-  public fun onReferenceLeaked(name: String, alive: Duration)
+public abstract class LeakListener {
+  public open fun onReferenceCollected(description: () -> String) {}
+  public open fun onReferenceLeaked(reference: Any, alive: Duration, description: () -> String) {}
 }
 
 internal class TimeBasedLeakDetector(
@@ -77,26 +77,27 @@ internal class TimeBasedLeakDetector(
   internal val gc = detectGc()
   private val watchedReferences = concurrentMutableListOf<WatchedReference>()
 
-  override fun watchReference(reference: Any, name: String) {
+  override fun watchReference(reference: Any, description: () -> String) {
     watchedReferences += WatchedReference(
-      name = name,
       weakReference = WeakReference(reference),
       watchedAt = timeSource.markNow(),
+      description = description,
     )
   }
 
   override suspend fun checkLeaks() {
     gc.collect()
 
     watchedReferences.removeIf { watchedReference ->
-      if (watchedReference.weakReference.get() == null) {
-        listener.onReferenceCollected(watchedReference.name)
+      val reference = watchedReference.weakReference.get()
+      if (reference == null) {
+        listener.onReferenceCollected(watchedReference.description)
         return@removeIf true
       }
 
       val alive = watchedReference.watchedAt.elapsedNow()
       if (alive >= leakThreshold) {
-        listener.onReferenceLeaked(watchedReference.name, alive)
+        listener.onReferenceLeaked(reference, alive, watchedReference.description)
         return@removeIf true
       }
 
@@ -105,8 +106,8 @@ internal class TimeBasedLeakDetector(
   }
 
   private class WatchedReference(
-    val name: String,
     val weakReference: WeakReference<Any>,
     val watchedAt: TimeMark,
+    val description: () -> String,
   )
 }

redwood-leak-detector/src/commonTest/kotlin/app/cash/redwood/leaks/LeakDetectorTest.kt

@@ -34,8 +34,10 @@ import kotlinx.coroutines.test.runTest
 class LeakDetectorTest {
   private val timeSource = TestTimeSource()
   private val listener = RecordingLeakListener()
-  private val leakDetector = LeakDetector.timeBased(listener, timeSource, 10.seconds)
-  private var ref: Any? = Any()
+  private val leakDetector = LeakDetector.timeBased(timeSource, 10.seconds, listener)
+  private var ref: Any? = object : Any() {
+    override fun toString() = "anAny"
+  }
 
   @BeforeTest fun before() {
     assertThat(leakDetector)
@@ -52,7 +54,7 @@ class LeakDetectorTest {
   }
 
   @Test fun detectImmediateCollection() = runTest {
-    leakDetector.watchReference(ref!!, "ref")
+    leakDetector.watchReference(ref!!) { "ref" }
 
     ref = null
     wait(10.seconds)
@@ -62,7 +64,7 @@ class LeakDetectorTest {
   }
 
   @Test fun detectDelayedCollection() = runTest {
-    leakDetector.watchReference(ref!!, "ref")
+    leakDetector.watchReference(ref!!) { "ref" }
 
     wait(5.seconds)
 
@@ -78,20 +80,20 @@ class LeakDetectorTest {
   }
 
   @Test fun detectLeak() = runTest {
-    leakDetector.watchReference(ref!!, "ref")
+    leakDetector.watchReference(ref!!) { "ref" }
 
     // Only advance virtual time for checking the event message.
     timeSource += 15.seconds
 
     leakDetector.checkLeaks()
-    assertThat(listener.events).containsExactly("leaked @ 15s: ref")
+    assertThat(listener.events).containsExactly("leaked anAny @ 15s: ref")
   }
 
   @Test fun concurrencyStressTest() = runTest {
     coroutineScope {
       repeat(10_000) { i ->
         launch(Dispatchers.Default) {
-          leakDetector.watchReference(Any(), "$i")
+          leakDetector.watchReference(Any()) { "$i" }
         }
       }
       repeat(100) {

redwood-leak-detector/src/commonTest/kotlin/app/cash/redwood/leaks/RecordingLeakListener.kt

@@ -17,14 +17,14 @@ package app.cash.redwood.leaks
 
 import kotlin.time.Duration
 
-class RecordingLeakListener : LeakListener {
+class RecordingLeakListener : LeakListener() {
   val events = mutableListOf<String>()
 
-  override fun onReferenceCollected(name: String) {
-    events += "collected: $name"
+  override fun onReferenceCollected(description: () -> String) {
+    events += "collected: ${description()}"
   }
 
-  override fun onReferenceLeaked(name: String, alive: Duration) {
-    events += "leaked @ $alive: $name"
+  override fun onReferenceLeaked(reference: Any, alive: Duration, description: () -> String) {
+    events += "leaked $reference @ $alive: ${description()}"
   }
 }

redwood-protocol-host/api/redwood-protocol-host.api

@@ -5,7 +5,7 @@ public abstract interface class app/cash/redwood/protocol/host/GeneratedProtocol
 }
 
 public final class app/cash/redwood/protocol/host/HostProtocolAdapter : app/cash/redwood/protocol/ChangesSink {
-	public synthetic fun <init> (Ljava/lang/String;Lapp/cash/redwood/widget/Widget$Children;Lapp/cash/redwood/protocol/host/ProtocolFactory;Lapp/cash/redwood/protocol/EventSink;Lkotlin/jvm/internal/DefaultConstructorMarker;)V
+	public synthetic fun <init> (Ljava/lang/String;Lapp/cash/redwood/widget/Widget$Children;Lapp/cash/redwood/protocol/host/ProtocolFactory;Lapp/cash/redwood/protocol/EventSink;Lapp/cash/redwood/leaks/LeakDetector;Lkotlin/jvm/internal/DefaultConstructorMarker;)V
 	public final fun close ()V
 	public fun sendChanges (Ljava/util/List;)V
 }
@@ -41,6 +41,7 @@ public abstract class app/cash/redwood/protocol/host/ProtocolNode {
 	public final fun getId-0HhLjSo ()I
 	public abstract fun getWidget ()Lapp/cash/redwood/widget/Widget;
 	public final fun getWidgetTag-BlhN7y0 ()I
+	public abstract fun toString ()Ljava/lang/String;
 	public final fun updateModifier (Lapp/cash/redwood/Modifier;)V
 	public abstract fun visitIds (Lkotlin/jvm/functions/Function1;)V
 }

redwood-protocol-host/api/redwood-protocol-host.klib.api

@@ -42,12 +42,13 @@ abstract class <#A: kotlin/Any> app.cash.redwood.protocol.host/ProtocolNode { //
     abstract fun apply(app.cash.redwood.protocol/PropertyChange, app.cash.redwood.protocol/EventSink) // app.cash.redwood.protocol.host/ProtocolNode.apply|apply(app.cash.redwood.protocol.PropertyChange;app.cash.redwood.protocol.EventSink){}[0]
     abstract fun children(app.cash.redwood.protocol/ChildrenTag): app.cash.redwood.protocol.host/ProtocolChildren<#A>? // app.cash.redwood.protocol.host/ProtocolNode.children|children(app.cash.redwood.protocol.ChildrenTag){}[0]
     abstract fun detach() // app.cash.redwood.protocol.host/ProtocolNode.detach|detach(){}[0]
+    abstract fun toString(): kotlin/String // app.cash.redwood.protocol.host/ProtocolNode.toString|toString(){}[0]
     abstract fun visitIds(kotlin/Function1<app.cash.redwood.protocol/Id, kotlin/Unit>) // app.cash.redwood.protocol.host/ProtocolNode.visitIds|visitIds(kotlin.Function1<app.cash.redwood.protocol.Id,kotlin.Unit>){}[0]
     final fun updateModifier(app.cash.redwood/Modifier) // app.cash.redwood.protocol.host/ProtocolNode.updateModifier|updateModifier(app.cash.redwood.Modifier){}[0]
 }
 
 final class <#A: kotlin/Any> app.cash.redwood.protocol.host/HostProtocolAdapter : app.cash.redwood.protocol/ChangesSink { // app.cash.redwood.protocol.host/HostProtocolAdapter|null[0]
-    constructor <init>(app.cash.redwood.protocol/RedwoodVersion, app.cash.redwood.widget/Widget.Children<#A>, app.cash.redwood.protocol.host/ProtocolFactory<#A>, app.cash.redwood.protocol/EventSink) // app.cash.redwood.protocol.host/HostProtocolAdapter.<init>|<init>(app.cash.redwood.protocol.RedwoodVersion;app.cash.redwood.widget.Widget.Children<1:0>;app.cash.redwood.protocol.host.ProtocolFactory<1:0>;app.cash.redwood.protocol.EventSink){}[0]
+    constructor <init>(app.cash.redwood.protocol/RedwoodVersion, app.cash.redwood.widget/Widget.Children<#A>, app.cash.redwood.protocol.host/ProtocolFactory<#A>, app.cash.redwood.protocol/EventSink, app.cash.redwood.leaks/LeakDetector) // app.cash.redwood.protocol.host/HostProtocolAdapter.<init>|<init>(app.cash.redwood.protocol.RedwoodVersion;app.cash.redwood.widget.Widget.Children<1:0>;app.cash.redwood.protocol.host.ProtocolFactory<1:0>;app.cash.redwood.protocol.EventSink;app.cash.redwood.leaks.LeakDetector){}[0]
 
     final fun close() // app.cash.redwood.protocol.host/HostProtocolAdapter.close|close(){}[0]
     final fun sendChanges(kotlin.collections/List<app.cash.redwood.protocol/Change>) // app.cash.redwood.protocol.host/HostProtocolAdapter.sendChanges|sendChanges(kotlin.collections.List<app.cash.redwood.protocol.Change>){}[0]

redwood-protocol-host/build.gradle

@@ -14,6 +14,7 @@ kotlin {
       dependencies {
         api projects.redwoodProtocol
         api projects.redwoodWidget
+        api projects.redwoodLeakDetector
       }
     }
     commonTest {

redwood-protocol-host/src/commonMain/kotlin/app/cash/redwood/protocol/host/HostProtocolAdapter.kt

@@ -17,6 +17,7 @@ package app.cash.redwood.protocol.host
 
 import app.cash.redwood.Modifier
 import app.cash.redwood.RedwoodCodegenApi
+import app.cash.redwood.leaks.LeakDetector
 import app.cash.redwood.protocol.Change
 import app.cash.redwood.protocol.ChangesSink
 import app.cash.redwood.protocol.ChildrenChange
@@ -51,6 +52,7 @@ public class HostProtocolAdapter<W : Any>(
   container: Widget.Children<W>,
   factory: ProtocolFactory<W>,
   private val eventSink: EventSink,
+  private val leakDetector: LeakDetector,
 ) : ChangesSink {
   private val factory = requireNotNull(factory as? GeneratedProtocolFactory<W>) {
     "Factory ${factory::class} was not generated by Redwood or is out of date"
@@ -183,13 +185,28 @@ public class HostProtocolAdapter<W : Any>(
       pool.addFirst(removedNode)
       if (pool.size > POOL_SIZE) {
         val evicted = pool.removeLast() // Evict the least-recently added element.
-        evicted.detach()
+        watchForLeaksAndDetach(evicted, "evicted from reuse pool")
       }
     } else {
-      removedNode.detach()
+      watchForLeaksAndDetach(removedNode, "not eligible for reuse")
     }
   }
 
+  private fun watchForLeaksAndDetach(node: ProtocolNode<W>, cause: String) {
+    leakDetector.watchReference(node.widget.value) {
+      "Widget's native UI view when $cause"
+    }
+    leakDetector.watchReference(node.widget) {
+      "Widget when $cause"
+    }
+    leakDetector.watchReference(node) {
+      "Redwood-internal widget protocol node when $cause"
+    }
+
+    // Detaching frees the node's reference to the widget, so this must be done last.
+    node.detach()
+  }
+
   /**
    * Implements widget reuse (view recycling).
    *
@@ -404,6 +421,8 @@ private class RootProtocolNode<W : Any>(
   override fun detach() {
     children.detach()
   }
+
+  override fun toString() = "RootProtocolNode"
 }
 
 private const val REUSE_MODIFIER_TAG = -4_543_827

redwood-protocol-host/src/commonMain/kotlin/app/cash/redwood/protocol/host/ProtocolNode.kt

@@ -67,8 +67,15 @@ public abstract class ProtocolNode<W : Any>(
   /** Recursively visit IDs in this widget's tree, starting with this widget's [id]. */
   public abstract fun visitIds(block: (Id) -> Unit)
 
-  /** Detach all child widgets recursively, then clear direct references to them. */
+  /**
+   * Detach all child widgets recursively, then clear direct references to them.
+   *
+   * After this is called there will be no further calls to this node.
+   */
   public abstract fun detach()
+
+  /** Human-readable name of this node along with [id] and [widgetTag]. */
+  public abstract override fun toString(): String
 }
 
 /**

redwood-protocol-host/src/commonTest/kotlin/app/cash/redwood/protocol/host/ChildrenNodeIndexTest.kt

@@ -141,6 +141,8 @@ private class WidgetNode(override val widget: StringWidget) : ProtocolNode<Strin
 
   override fun detach() {
   }
+
+  override fun toString() = "WidgetNode"
 }
 
 private class StringWidget(override val value: String) : Widget<String> {