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/commonMain/kotlin/app/cash/redwood/leaks/zipline/LeakDetectorTestService.kt

@@ -18,7 +18,7 @@ package app.cash.redwood.leaks.zipline
 import app.cash.zipline.ZiplineService
 
 interface LeakDetectorTestService : ZiplineService {
-  fun leakDetectorDisabled()
+  suspend fun leakDetectorDisabled()
 
   // TODO Once QuickJS supports WeakRef, enable regular tests:
   //  suspend fun detectImmediateCollection()

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

@@ -16,28 +16,26 @@
 package app.cash.redwood.leaks.zipline
 
 import app.cash.redwood.leaks.LeakDetector
-import app.cash.redwood.leaks.LeakListener
 import app.cash.redwood.leaks.zipline.LeakDetectorTestService.Companion.SERVICE_NAME
 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
+import kotlinx.coroutines.coroutineScope
 
 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) {}
-      },
+  override suspend fun leakDetectorDisabled() = coroutineScope {
+    val leakDetector = LeakDetector.timeBasedIn(
+      scope = this,
       timeSource = TimeSource.Monotonic,
-      leakThreshold = 2.seconds,
+      leakThreshold = 0.seconds,
+      callback = { _, _ -> throw AssertionError() },
     )
+
     // 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.
-    assertThat(leakDetector).isSameInstanceAs(LeakDetector.None)
+    leakDetector.watchReference(this@LeakDetectorTestServiceImpl, "")
+
+    leakDetector.awaitClose()
   }
 }
 

redwood-leak-detector-zipline-test/src/hostTest/kotlin/app/cash/redwood/leaks/zipline/LeakDetectorZiplineTest.kt

@@ -73,7 +73,7 @@ class LeakDetectorZiplineTest {
     zipline.close()
   }
 
-  @Test fun leakDetectorDisabled() {
+  @Test fun leakDetectorDisabled() = runBlocking {
     service.leakDetectorDisabled()
   }
 }

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

@@ -1,22 +1,17 @@
-public abstract interface class app/cash/redwood/leaks/LeakDetector {
+public abstract interface class app/cash/redwood/leaks/LeakDetector : java/lang/AutoCloseable {
 	public static final field Companion Lapp/cash/redwood/leaks/LeakDetector$Companion;
-	public abstract fun checkLeaks (Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
+	public abstract fun awaitClose (Lkotlin/coroutines/Continuation;)Ljava/lang/Object;
+	public abstract fun close ()V
 	public abstract fun watchReference (Ljava/lang/Object;Ljava/lang/String;)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 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 abstract interface class app/cash/redwood/leaks/LeakDetector$Callback {
+	public abstract fun onReferenceLeaked (Ljava/lang/Object;Ljava/lang/String;)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 final class app/cash/redwood/leaks/LeakDetector$Companion {
+	public final fun none ()Lapp/cash/redwood/leaks/LeakDetector;
+	public final fun timeBasedIn-exY8QGI (Lkotlinx/coroutines/CoroutineScope;Lkotlin/time/TimeSource;JLapp/cash/redwood/leaks/LeakDetector$Callback;)Lapp/cash/redwood/leaks/LeakDetector;
 }
 
 public abstract interface annotation class app/cash/redwood/leaks/RedwoodLeakApi : java/lang/annotation/Annotation {

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

@@ -10,21 +10,17 @@ open annotation class app.cash.redwood.leaks/RedwoodLeakApi : kotlin/Annotation
     constructor <init>() // app.cash.redwood.leaks/RedwoodLeakApi.<init>|<init>(){}[0]
 }
 
-abstract interface app.cash.redwood.leaks/LeakDetector { // app.cash.redwood.leaks/LeakDetector|null[0]
+abstract interface app.cash.redwood.leaks/LeakDetector : kotlin/AutoCloseable { // app.cash.redwood.leaks/LeakDetector|null[0]
+    abstract fun close() // app.cash.redwood.leaks/LeakDetector.close|close(){}[0]
     abstract fun watchReference(kotlin/Any, kotlin/String) // app.cash.redwood.leaks/LeakDetector.watchReference|watchReference(kotlin.Any;kotlin.String){}[0]
-    abstract suspend fun checkLeaks() // app.cash.redwood.leaks/LeakDetector.checkLeaks|checkLeaks(){}[0]
+    abstract suspend fun awaitClose() // app.cash.redwood.leaks/LeakDetector.awaitClose|awaitClose(){}[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]
+    abstract fun interface Callback { // app.cash.redwood.leaks/LeakDetector.Callback|null[0]
+        abstract fun onReferenceLeaked(kotlin/Any, kotlin/String) // app.cash.redwood.leaks/LeakDetector.Callback.onReferenceLeaked|onReferenceLeaked(kotlin.Any;kotlin.String){}[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 suspend fun checkLeaks() // app.cash.redwood.leaks/LeakDetector.None.checkLeaks|checkLeaks(){}[0]
+    final object Companion { // app.cash.redwood.leaks/LeakDetector.Companion|null[0]
+        final fun none(): app.cash.redwood.leaks/LeakDetector // app.cash.redwood.leaks/LeakDetector.Companion.none|none(){}[0]
+        final fun timeBasedIn(kotlinx.coroutines/CoroutineScope, kotlin.time/TimeSource, kotlin.time/Duration, app.cash.redwood.leaks/LeakDetector.Callback): app.cash.redwood.leaks/LeakDetector // app.cash.redwood.leaks/LeakDetector.Companion.timeBasedIn|timeBasedIn(kotlinx.coroutines.CoroutineScope;kotlin.time.TimeSource;kotlin.time.Duration;app.cash.redwood.leaks.LeakDetector.Callback){}[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]
-}

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

@@ -17,7 +17,7 @@ package app.cash.redwood.leaks
 
 internal expect fun detectGc(): Gc
 
-internal interface Gc {
+internal fun interface Gc {
   suspend fun collect()
 
   object None : Gc {

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

@@ -15,98 +15,140 @@
  */
 package app.cash.redwood.leaks
 
+import app.cash.redwood.leaks.LeakDetector.Callback
 import kotlin.time.Duration
-import kotlin.time.TimeMark
 import kotlin.time.TimeSource
+import kotlinx.coroutines.CoroutineScope
+import kotlinx.coroutines.CoroutineStart.UNDISPATCHED
+import kotlinx.coroutines.Job
+import kotlinx.coroutines.cancel
+import kotlinx.coroutines.delay
+import kotlinx.coroutines.flow.SharingStarted
+import kotlinx.coroutines.flow.flow
+import kotlinx.coroutines.flow.shareIn
+import kotlinx.coroutines.launch
 
 /** Watch references and detect when they leak. */
 @RedwoodLeakApi
-public interface LeakDetector {
+public interface LeakDetector : AutoCloseable {
   /**
    * Add [reference] as a watched instance that is expected to be garbage collected soon.
    *
    * This function is safe to call from any thread.
+   *
+   * @param note Information about why [reference] is being watched.
    */
-  public fun watchReference(reference: Any, name: String)
+  public fun watchReference(reference: Any, note: String)
 
   /**
-   * Trigger garbage collection and determine if any watched references have leaked per this
-   * instance's leak policy.
-   *
-   * This function is safe to call from any thread.
+   * Suspend until all current watched references are either collected or detected as a leak, and
+   * throw an exception for any new calls to [watchReference].
    */
-  public suspend fun checkLeaks()
+  public suspend fun awaitClose()
 
-  public object None : LeakDetector {
-    override fun watchReference(reference: Any, name: String) {}
-    override suspend fun checkLeaks() {}
-  }
+  /**
+   * Immediately stop all leak detection of current watched references, and throw an exception for
+   * any new calls to [watchReference].
+   */
+  override fun close()
 
   public companion object {
     /**
-     * Create a time-based [LeakDetector] which reports leaks to [listener] if watched references
+     * Create a time-based [LeakDetector] which reports leaks if watched references
      * have not been garbage collected before the [leakThreshold].
      *
-     * This function may return [None] if the platform does not support weak references, in which
-     * case [listener] will never be invoked.
+     * This function may return a [no-op detector][none] if the platform does not support weak references.
      */
-    public fun timeBased(
-      listener: LeakListener,
+    public fun timeBasedIn(
+      scope: CoroutineScope,
       timeSource: TimeSource,
       leakThreshold: Duration,
+      callback: Callback,
     ): LeakDetector {
       if (hasWeakReference()) {
-        return TimeBasedLeakDetector(listener, timeSource, leakThreshold)
+        return TimeBasedLeakDetector(scope, detectGc(), timeSource, leakThreshold, callback)
       }
-      return None
+      return none()
     }
+
+    /** A [LeakDetector] that does not watch references for leaks. */
+    public fun none(): LeakDetector = NoOpLeakDetector()
   }
-}
 
-@RedwoodLeakApi
-public interface LeakListener {
-  public fun onReferenceCollected(name: String)
-  public fun onReferenceLeaked(name: String, alive: Duration)
+  public fun interface Callback {
+    public fun onReferenceLeaked(reference: Any, note: String)
+  }
 }
 
 internal class TimeBasedLeakDetector(
-  private val listener: LeakListener,
+  private val scope: CoroutineScope,
+  internal val gc: Gc,
   private val timeSource: TimeSource,
   private val leakThreshold: Duration,
+  private val callback: Callback,
 ) : LeakDetector {
-  internal val gc = detectGc()
-  private val watchedReferences = concurrentMutableListOf<WatchedReference>()
-
-  override fun watchReference(reference: Any, name: String) {
-    watchedReferences += WatchedReference(
-      name = name,
-      weakReference = WeakReference(reference),
-      watchedAt = timeSource.markNow(),
-    )
-  }
+  private var closed = false
 
-  override suspend fun checkLeaks() {
-    gc.collect()
+  private val checkJob = Job()
+  private val checkTickle = flow {
+    val checkPeriod = leakThreshold / 2
+    while (true) {
+      delay(checkPeriod)
+      gc.collect()
+      emit(Unit)
+    }
+  }.shareIn(
+    scope = CoroutineScope(scope.coroutineContext + checkJob),
+    started = SharingStarted.WhileSubscribed(),
+  )
 
-    watchedReferences.removeIf { watchedReference ->
-      if (watchedReference.weakReference.get() == null) {
-        listener.onReferenceCollected(watchedReference.name)
-        return@removeIf true
-      }
+  private val watchJob = Job()
 
-      val alive = watchedReference.watchedAt.elapsedNow()
-      if (alive >= leakThreshold) {
-        listener.onReferenceLeaked(watchedReference.name, alive)
-        return@removeIf true
-      }
+  override fun watchReference(reference: Any, note: String) {
+    check(!closed) { "closed" }
+    internalWatch(WeakReference(reference), note)
+  }
 
-      false
+  private fun internalWatch(weakReference: WeakReference<Any>, reason: String) {
+    scope.launch(watchJob, start = UNDISPATCHED) {
+      val watchedAt = timeSource.markNow()
+      checkTickle.collect {
+        val reference = weakReference.get() ?: cancel()
+        if (watchedAt.elapsedNow() >= leakThreshold) {
+          callback.onReferenceLeaked(reference, reason)
+          cancel()
+        }
+      }
     }
   }
 
-  private class WatchedReference(
-    val name: String,
-    val weakReference: WeakReference<Any>,
-    val watchedAt: TimeMark,
-  )
+  override suspend fun awaitClose() {
+    closed = true
+    // Wait for all active watches to complete.
+    watchJob.children.forEach { it.join() }
+    watchJob.cancel()
+    checkJob.cancel()
+  }
+
+  override fun close() {
+    closed = true
+    watchJob.cancel()
+    checkJob.cancel()
+  }
+}
+
+private class NoOpLeakDetector : LeakDetector {
+  private var closed = false
+
+  override fun watchReference(reference: Any, note: String) {
+    check(!closed) { "closed" }
+  }
+
+  override suspend fun awaitClose() {
+    closed = true
+  }
+
+  override fun close() {
+    closed = true
+  }
 }