Skip to content

Commit 09d400e

Browse files
authored
Add a hook to close EventListener.Factory instances (#2280)
* Add a hook to close EventListener.Factory instances These things potentially hold resources and so should be closeable. * Track API changes
1 parent 6758e0c commit 09d400e

File tree

16 files changed

+72
-14
lines changed

16 files changed

+72
-14
lines changed

CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
[Unreleased]: https://github.com/cashapp/redwood/compare/0.13.0...HEAD
55

66
New:
7-
- Nothing yet!
7+
- `EventListener.Factory.close()` is called by `TreehouseApp.close()` to release any resources held by the factory.
88

99
Changed:
1010
- Nothing yet!

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ public final class app/cash/redwood/treehouse/EventListener$Companion {
6464
public final fun getNONE ()Lapp/cash/redwood/treehouse/EventListener$Factory;
6565
}
6666

67-
public abstract interface class app/cash/redwood/treehouse/EventListener$Factory {
67+
public abstract interface class app/cash/redwood/treehouse/EventListener$Factory : java/lang/AutoCloseable {
6868
public abstract fun create (Lapp/cash/redwood/treehouse/TreehouseApp;Ljava/lang/String;)Lapp/cash/redwood/treehouse/EventListener;
6969
}
7070

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ public final class app/cash/redwood/treehouse/EventListener$Companion {
6464
public final fun getNONE ()Lapp/cash/redwood/treehouse/EventListener$Factory;
6565
}
6666

67-
public abstract interface class app/cash/redwood/treehouse/EventListener$Factory {
67+
public abstract interface class app/cash/redwood/treehouse/EventListener$Factory : java/lang/AutoCloseable {
6868
public abstract fun create (Lapp/cash/redwood/treehouse/TreehouseApp;Ljava/lang/String;)Lapp/cash/redwood/treehouse/EventListener;
6969
}
7070

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ open class app.cash.redwood.treehouse/EventListener { // app.cash.redwood.treeho
175175
open fun unknownWidget(app.cash.redwood.protocol/WidgetTag) // app.cash.redwood.treehouse/EventListener.unknownWidget|unknownWidget(app.cash.redwood.protocol.WidgetTag){}[0]
176176
open fun ziplineCreated(app.cash.zipline/Zipline) // app.cash.redwood.treehouse/EventListener.ziplineCreated|ziplineCreated(app.cash.zipline.Zipline){}[0]
177177

178-
abstract fun interface Factory { // app.cash.redwood.treehouse/EventListener.Factory|null[0]
178+
abstract interface Factory : kotlin/AutoCloseable { // app.cash.redwood.treehouse/EventListener.Factory|null[0]
179179
abstract fun create(app.cash.redwood.treehouse/TreehouseApp<*>, kotlin/String?): app.cash.redwood.treehouse/EventListener // app.cash.redwood.treehouse/EventListener.Factory.create|create(app.cash.redwood.treehouse.TreehouseApp<*>;kotlin.String?){}[0]
180180
}
181181

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,25 @@ class LeaksTest {
127127
eventListenerLeakWatcher.assertNotLeaked()
128128
}
129129

130+
@Test
131+
fun eventListenerFactoryNotLeaked() = runTest {
132+
val tester = TreehouseTester(this)
133+
tester.eventListenerFactory = RetainEverythingEventListenerFactory(tester.eventLog)
134+
val treehouseApp = tester.loadApp()
135+
136+
val eventListenerFactoryLeakWatcher = LeakWatcher {
137+
tester.eventListenerFactory
138+
}
139+
140+
// Stop referencing our EventListener.Factory from our test harness.
141+
tester.eventListenerFactory = FakeEventListener.Factory(tester.eventLog)
142+
143+
// After close, it's unreachable.
144+
treehouseApp.close()
145+
eventListenerFactoryLeakWatcher.assertNotLeaked()
146+
tester.eventLog.takeEvent("EventListener.Factory.close()", skipOthers = true)
147+
}
148+
130149
@Test
131150
fun contentSourceNotLeaked() = runTest {
132151
val tester = TreehouseTester(this)

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,4 +45,8 @@ class RetainEverythingEventListenerFactory(
4545
override fun codeUnloaded() {
4646
eventLog += "codeUnloaded"
4747
}
48+
49+
override fun close() {
50+
eventLog += "EventListener.Factory.close()"
51+
}
4852
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,5 +43,6 @@ class TreehouseTesterTest {
4343
assertThat(buttonValue.text).isEqualTo("This is TreehouseTesterTestHappyPathStep2")
4444

4545
treehouseApp.stop()
46+
treehouseApp.close()
4647
}
4748
}

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ internal abstract class CodeHost<A : AppService>(
106106
codeUpdatesScope.collectCodeUpdates(eventListenerFactory)
107107
}
108108

109-
/** This function may only be invoked on [TreehouseDispatchers.zipline]. */
109+
/** This function may only be invoked on [TreehouseDispatchers.ui]. */
110110
fun stop() {
111111
dispatchers.checkUi()
112112

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

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,6 @@ import app.cash.redwood.protocol.Id
2121
import app.cash.redwood.protocol.ModifierTag
2222
import app.cash.redwood.protocol.PropertyTag
2323
import app.cash.redwood.protocol.WidgetTag
24-
import app.cash.redwood.treehouse.EventListener.Factory
2524
import app.cash.zipline.Call
2625
import app.cash.zipline.CallResult
2726
import app.cash.zipline.Zipline
@@ -367,7 +366,15 @@ public open class EventListener {
367366
) {
368367
}
369368

370-
public fun interface Factory {
369+
/**
370+
* Creates new instances of [EventListener].
371+
*
372+
* A new [EventListener] will be created for each code load attempt: there may be multiple
373+
* instances for a single [TreehouseApp] due to hot reloading.
374+
*
375+
* This factory will be closed when the [TreehouseApp] it belongs to is closed.
376+
*/
377+
public interface Factory : AutoCloseable {
371378
/**
372379
* Returns an event listener that receives the events of a specific code session. Each code
373380
* session includes a single [Zipline] instance, unless code loading fails, in which case there
@@ -380,8 +387,10 @@ public open class EventListener {
380387
}
381388

382389
public companion object {
383-
public val NONE: Factory = Factory { _, _ ->
384-
EventListener()
390+
public val NONE: Factory = object : Factory {
391+
override fun create(app: TreehouseApp<*>, manifestUrl: String?) = EventListener()
392+
override fun close() {
393+
}
385394
}
386395
}
387396
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,7 @@ internal class RealTreehouseApp<A : AppService> private constructor(
177177

178178
closed = true
179179
spec = null
180+
eventListenerFactory?.close()
180181
eventListenerFactory = null
181182
stop()
182183
dispatchers.close()

0 commit comments

Comments
 (0)