Detect host-side node, widget, and view leaks #2254
Conversation
JakeWharton
force-pushed
the
jw.leak-integration.2024-08-13
branch
4 times, most recently
from
cc24296 to
4b6edaa
Compare
redwood-treehouse-host/src/commonMain/kotlin/app/cash/redwood/treehouse/TreehouseAppContent.kt
Outdated
Show resolved
Hide resolved
redwood-leak-detector/src/commonMain/kotlin/app/cash/redwood/leaks/LeakDetector.kt
Outdated
Show resolved
Hide resolved
| "Redwood-internal widget protocol node when $cause" | ||
| } | ||
|
|
||
| // Detaching frees the node's reference to the widget, so this must be done last. |
| .addModifiers(OVERRIDE) | ||
| .returns(STRING) | ||
| // This explicit string builder usage allows sharing of strings in dex. | ||
| // See https://jakewharton.com/the-economics-of-generated-code/#string-duplication. |
| timeSource = TimeSource.Monotonic, | ||
| leakThreshold = 10.seconds, | ||
| listener = object : LeakListener() { | ||
| override fun onReferenceLeaked( |
There was a problem hiding this comment.
I’d like to somehow wire things together so that this is just another event to listen to in EventListener
There was a problem hiding this comment.
(And we’d need to hoist the policy up to the TreehouseApp.Factory or something)
There was a problem hiding this comment.
I originally had this design, but it meant that the ownership of creating the LeakDetector and running its coroutine are deep inside the code.
I think maybe the listener of whether something leaks should be supplied when you watch something. This allows the decision of whether the leak detector is real or not to be very far away from the "watch this and notify me" call.
There was a problem hiding this comment.
Yeah I don't think this makes sense. The EventListener is scoped to a code load. We can use that for reporting guest-side leaks, but it's not a good fit for host-side. Host-side leaks need to monitor things like the Zipline instance whose lifetime exceeds that of the EventListener. Since we null the real listener on cancel (to avoid a leak), we'd have nowhere to report actual leaks. I think for now I'm going to leave the whole LeakDetector exposed through the factory.
There was a problem hiding this comment.
Once we get guest-side leaks, they'll come through the EventListener.
JakeWharton
force-pushed
the
jw.leak-integration.2024-08-13
branch
2 times, most recently
from
fd2ec00 to
c47f8dc
Compare
|
This is ready but it's 5pm and I'm not starting a rebase. Tomorrow problem. |
JakeWharton
force-pushed
the
jw.leak-integration.2024-08-13
branch
2 times, most recently
from
830fef6 to
f5e2860
Compare
| */ | ||
| public suspend fun checkLeaks() | ||
| public suspend fun awaitClose() |
|
|
||
| leakDetector.checkLeaks() | ||
| assertThat(listener.events).isEmpty() | ||
| // No refs? No GC. |
JakeWharton
force-pushed
the
jw.leak-integration.2024-08-13
branch
from
f5e2860 to
cc09030
Compare
Refs #575.
CHANGELOG.md's "Unreleased" section has been updated, if applicable.