mirror of
https://github.com/cashapp/redwood.git
synced 2026-03-09 21:58:16 +08:00
Fix memory leaks in Redwood UIView components (#2899)
* Fix memory leaks in UIView layout components Use WeakReference to break retain cycles in UIViewBox and UIViewFlexContainer. In Kotlin/Native, lambdas and anonymous objects create strong references by default. The SizeListener callbacks were capturing their parent containers strongly, creating retain cycles that prevented deallocation: - UIViewBox.View → children → insert lambda → SizeListener → View - UIViewFlexContainer → NodeSizeListener → enclosing container This caused UI components to remain in memory after they should have been deallocated (e.g., after logout), along with their associated graphics buffers (VM: Memory Tag objects). The fix uses WeakReference for the captured parent references, allowing proper cleanup when the parent containers are no longer needed. * Fix memory leak in RedwoodUIView sizeListener Use WeakReference to break retain cycle in RedwoodUIView's sizeListener. The sizeListener was capturing valueRootView strongly, creating a retain cycle: RedwoodUIView → valueRootView → sizeListener → valueRootView This prevented RedwoodUIView instances from being deallocated after they were removed from the view hierarchy. The fix uses WeakReference for the captured valueRootView reference, allowing proper cleanup when the view is no longer needed. * Fix memory leak: Clear widgetSystem in HostProtocolAdapter.close() The HostProtocolAdapter was retaining a reference to the WidgetSystem even after close() was called. This created a leak chain: HostProtocolAdapter (Kotlin) → WidgetSystem (Kotlin) → RealTreehouseWidgetFactory (Swift) → ContentListener (Swift) This prevented Swift objects from being deallocated even though they had no strong references visible in the memory graph, because they were being retained by Kotlin/Native's reference counting. The fix makes widgetSystem nullable and clears it in close(), breaking the Kotlin→Swift reference chain and allowing proper cleanup. * Add CHANGELOG items for the leaks
This commit is contained in:
@@ -1,5 +1,13 @@
|
||||
# Change Log
|
||||
|
||||
## Unreleased
|
||||
|
||||
Fixed:
|
||||
- Fix memory leaks in UIView layout components (`UIViewBox`, `UIViewFlexContainer`, `RedwoodUIView`)
|
||||
- Fix memory leak in `HostProtocolAdapter`
|
||||
|
||||
These leaks affected some Treehouse-based screens and prevented UI components and their associated memory from being released after dismissal.
|
||||
|
||||
## [0.19.0]
|
||||
[0.19.0]: https://github.com/cashapp/redwood/releases/tag/0.19.0
|
||||
|
||||
|
||||
@@ -31,6 +31,8 @@ import app.cash.redwood.ui.Margin
|
||||
import app.cash.redwood.widget.ResizableWidget
|
||||
import app.cash.redwood.widget.UIViewChildren
|
||||
import app.cash.redwood.widget.Widget
|
||||
import kotlin.experimental.ExperimentalNativeApi
|
||||
import kotlin.native.ref.WeakReference
|
||||
import kotlinx.cinterop.CValue
|
||||
import kotlinx.cinterop.convert
|
||||
import kotlinx.cinterop.readValue
|
||||
@@ -89,13 +91,15 @@ internal class UIViewBox :
|
||||
var sizeListener: ResizableWidget.SizeListener? = null
|
||||
private val measurer = Measurer()
|
||||
|
||||
@OptIn(ExperimentalNativeApi::class)
|
||||
val children = UIViewChildren(
|
||||
container = this,
|
||||
insert = { index, widget ->
|
||||
if (widget is ResizableWidget<*>) {
|
||||
val weakView = WeakReference(this@View)
|
||||
widget.sizeListener = object : ResizableWidget.SizeListener {
|
||||
override fun invalidateSize() {
|
||||
this@View.invalidateSize()
|
||||
weakView.get()?.invalidateSize()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -27,6 +27,8 @@ import app.cash.redwood.widget.ResizableWidget.SizeListener
|
||||
import app.cash.redwood.widget.UIViewChildren
|
||||
import app.cash.redwood.yoga.FlexDirection
|
||||
import app.cash.redwood.yoga.Node
|
||||
import kotlin.experimental.ExperimentalNativeApi
|
||||
import kotlin.native.ref.WeakReference
|
||||
import kotlinx.cinterop.convert
|
||||
import platform.UIKit.UIView
|
||||
import platform.darwin.NSInteger
|
||||
@@ -119,15 +121,18 @@ private fun Node(view: UIView): Node {
|
||||
return result
|
||||
}
|
||||
|
||||
@OptIn(ExperimentalNativeApi::class)
|
||||
private class NodeSizeListener(
|
||||
private val node: Node,
|
||||
private val view: UIView,
|
||||
private val enclosing: UIViewFlexContainer,
|
||||
enclosing: UIViewFlexContainer,
|
||||
) : SizeListener {
|
||||
private val weakEnclosing = WeakReference(enclosing)
|
||||
|
||||
override fun invalidateSize() {
|
||||
if (node.markDirty()) {
|
||||
view.setNeedsLayout()
|
||||
enclosing.invalidateSize()
|
||||
weakEnclosing.get()?.invalidateSize()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -49,7 +49,7 @@ public class HostProtocolAdapter<W : Any>(
|
||||
guestVersion: RedwoodVersion,
|
||||
container: Widget.Children<W>,
|
||||
protocol: HostProtocol,
|
||||
private val widgetSystem: WidgetSystem<W>,
|
||||
widgetSystem: WidgetSystem<W>,
|
||||
private val eventSink: UiEventSink,
|
||||
private val leakDetector: LeakDetector,
|
||||
) : UiChangesSink {
|
||||
@@ -67,6 +67,8 @@ public class HostProtocolAdapter<W : Any>(
|
||||
/** Nodes available for reuse. */
|
||||
private val pool = ArrayDeque<ProtocolNode<W>>()
|
||||
|
||||
private var widgetSystem: WidgetSystem<W>? = widgetSystem
|
||||
|
||||
private var closed = false
|
||||
|
||||
override fun sendChanges(changes: List<UiChange>) {
|
||||
@@ -81,6 +83,7 @@ public class HostProtocolAdapter<W : Any>(
|
||||
when (change) {
|
||||
is UiCreate -> {
|
||||
val widgetProtocol = protocol.widget(change.tag) ?: continue
|
||||
val widgetSystem = this.widgetSystem ?: return
|
||||
val node = widgetProtocol.createNode(id, widgetSystem)
|
||||
val old = nodes.put(change.id.value, node)
|
||||
require(old == null) {
|
||||
@@ -126,6 +129,7 @@ public class HostProtocolAdapter<W : Any>(
|
||||
val node = node(id)
|
||||
node.reuse = change.reuse
|
||||
|
||||
val widgetSystem = this.widgetSystem ?: return
|
||||
change.modifier.forEachUnscoped { element ->
|
||||
widgetSystem.apply(node.widget.value, element)
|
||||
}
|
||||
@@ -178,6 +182,9 @@ public class HostProtocolAdapter<W : Any>(
|
||||
node.detach()
|
||||
}
|
||||
pool.clear()
|
||||
|
||||
// Clear the widget system to break the reference to Swift objects.
|
||||
widgetSystem = null
|
||||
}
|
||||
|
||||
private fun poolOrDetach(removedNode: ProtocolNode<W>) {
|
||||
|
||||
@@ -24,6 +24,8 @@ import app.cash.redwood.ui.OnBackPressedCallback
|
||||
import app.cash.redwood.ui.OnBackPressedDispatcher
|
||||
import app.cash.redwood.ui.Size
|
||||
import app.cash.redwood.ui.UiConfiguration
|
||||
import kotlin.experimental.ExperimentalNativeApi
|
||||
import kotlin.native.ref.WeakReference
|
||||
import kotlinx.cinterop.CValue
|
||||
import kotlinx.cinterop.cValue
|
||||
import kotlinx.cinterop.convert
|
||||
@@ -60,15 +62,19 @@ public open class RedwoodUIView : RedwoodView<UIView> {
|
||||
override val value: UIView
|
||||
get() = valueRootView
|
||||
|
||||
@OptIn(ExperimentalNativeApi::class)
|
||||
private val sizeListener = object : ResizableWidget.SizeListener {
|
||||
private val weakRootView = WeakReference(valueRootView)
|
||||
|
||||
override fun invalidateSize() {
|
||||
val rootView = weakRootView.get() ?: return
|
||||
// This view's size may have changed.
|
||||
valueRootView.setNeedsLayout() // For autolayout.
|
||||
valueRootView.invalidateIntrinsicContentSize() // For SwiftUI.
|
||||
rootView.setNeedsLayout() // For autolayout.
|
||||
rootView.invalidateIntrinsicContentSize() // For SwiftUI.
|
||||
|
||||
// And the superview should redo its layout also, if it exists.
|
||||
valueRootView.superview?.setNeedsLayout() // For autolayout.
|
||||
valueRootView.superview?.invalidateIntrinsicContentSize() // For SwiftUI.
|
||||
rootView.superview?.setNeedsLayout() // For autolayout.
|
||||
rootView.superview?.invalidateIntrinsicContentSize() // For SwiftUI.
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user