-
Notifications
You must be signed in to change notification settings - Fork 93
Fix memory leaks in Redwood UIView components #2899
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| var sizeListener: ResizableWidget.SizeListener? = null | ||
| private val measurer = Measurer() | ||
|
|
||
| @OptIn(kotlin.experimental.ExperimentalNativeApi::class) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not clear if this is an okay thing to do, and I'm hoping that someone who knows this area better than I can weigh in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems reasonable to me--WeakReference was added as an internal ExperimentalNativeApi in #2191 cc @JakeWharton
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep it should be fine to use ExperimentalNativeApis as an implementation detail (like here).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome. Thanks for verifying!
|
Build methodology, if you wanted to try this out locally: 1. Configure cash-treehouse to use local RedwoodCreate or edit # Enable Kotlin/Native to build iOS framework
treehouse.kotlin.native.enabled=true
# Point to your local Redwood repository
redwoodPath=/Users/jholliday/Development/redwood2. Build cash-treehouse with local Redwood changescd ~/Development/cash-treehouse
./gradlew debugSpm --no-configuration-cacheThis will:
3. Resolve SPM dependencies in cash-ioscd ~/Development/cash-ios
cash spm resolve4. Build and run Cash iOSBuild and run from Xcode |
| private val enclosing: UIViewFlexContainer, | ||
| enclosing: UIViewFlexContainer, | ||
| ) : SizeListener { | ||
| private val weakEnclosing = WeakReference(enclosing) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ooh, great catch here, this would be a strict retain cycle otherwise
dnagler
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes make sense to me! I think it's worth getting at least one more set of eyes on them just to be safe though.
Before merging, it'd be good to add a note in CHANGELOG.md. I think @colinrtwhite should be able to create a 0.19.1 release?
Added a 0.19.1 section in the Changelog |
dellisd
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me
redwood-layout-uiview/src/commonMain/kotlin/app/cash/redwood/layout/uiview/UIViewBox.kt
Outdated
Show resolved
Hide resolved
colinrtwhite
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, though I'm curious why the size listeners aren't being freed when the associated UI component is removed from the view hierarchy - maybe some ARC intricacies I don't understand.
HostProtocolAdapter - The widgetSystem was being retained even after close() was called, creating a Kotlin→Swift reference chain that kept Swift objects alive
Is it possible that this change is enough to fix the issue on its own?
| var sizeListener: ResizableWidget.SizeListener? = null | ||
| private val measurer = Measurer() | ||
|
|
||
| @OptIn(kotlin.experimental.ExperimentalNativeApi::class) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep it should be fine to use ExperimentalNativeApis as an implementation detail (like here).
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.
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.
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.
bb8d4c1 to
4ce075f
Compare
JakeWharton
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious as to why the built-in leak detection didn't catch these cases. Normally I'd want that blind spot fixed to prevent regression, but it's probably not worth the effort at this point.
CHANGELOG.md
Outdated
| ## [0.19.1] - 2026-01-08 | ||
| [0.19.1]: https://github.com/cashapp/redwood/releases/tag/0.19.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ## [0.19.1] - 2026-01-08 | |
| [0.19.1]: https://github.com/cashapp/redwood/releases/tag/0.19.1 | |
| ## Unreleased |
Normally there's already an unreleased section here, but I didn't add it when releasing 0.19.0 because I don't expect there to ever be another public release. The version, date, and link are inserted as part of the release process.
You're probably better off just doing an internal release to use this and any future changes. I would not bother doing public releases. But I'm still supportive of documenting the changes using the normal process.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
CHANGELOG.md
Outdated
| - Fix memory leaks in UIView layout components (`UIViewBox`, `UIViewFlexContainer`, `RedwoodUIView`) where `SizeListener` callbacks were creating retain cycles by capturing parent containers strongly. Now use `WeakReference` to break these cycles. | ||
| - Fix memory leak in `HostProtocolAdapter` where the `widgetSystem` was retained after `close()` was called, creating a Kotlin→Swift reference chain that prevented Swift objects from being deallocated. The `widgetSystem` is now cleared in `close()` to break this chain. | ||
|
|
||
| These leaks affected some Treehouse-based screens and prevented UI components and their associated memory from being released after dismissal. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| - Fix memory leaks in UIView layout components (`UIViewBox`, `UIViewFlexContainer`, `RedwoodUIView`) where `SizeListener` callbacks were creating retain cycles by capturing parent containers strongly. Now use `WeakReference` to break these cycles. | |
| - Fix memory leak in `HostProtocolAdapter` where the `widgetSystem` was retained after `close()` was called, creating a Kotlin→Swift reference chain that prevented Swift objects from being deallocated. The `widgetSystem` is now cleared in `close()` to break this chain. | |
| These leaks affected some Treehouse-based screens and prevented UI components and their associated memory from being released after dismissal. | |
| - Fix memory leaks in UIView layout components (`UIViewBox`, `UIViewFlexContainer`, `RedwoodUIView`) where `SizeListener` callbacks were creating retain cycles by capturing parent containers strongly. | |
| - Fix memory leak in `HostProtocolAdapter` where the `widgetSystem` was retained after `close()` was called, creating a Kotlin→Swift reference chain that prevented Swift objects from being deallocated. |
This log is for users who aren't expected to know implementation details. Those are better served in the commit message for us developers.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Condensed. The same details are indeed in the individual commits.
4ce075f to
e8c07bd
Compare
Yeah, probably not. Thank you for taking a look at this. It's very much appreciated 🙏 . |

This PR fixes multiple memory leaks in Redwood's iOS/UIView implementation that prevented UI components from being deallocated after they were removed from the view hierarchy (e.g., after logout).
I am not a Kotlin, GC, or Treehouse expert. This is the result of a local redwood -> cash-ios build, a test case of opening several receipts, and a conversation with an LLM that can see both sets of source about the references to leaked objects I can see using the iOS memory graph.
This does get rid of a large number of leaked objects on the iOS side.
Root Cause
Kotlin/Native uses reference counting similar to ARC, but lambdas and anonymous objects create strong references by default. Several components were creating retain cycles that prevented proper cleanup.
Fixes
UIViewBox and UIViewFlexContainer - Used WeakReference to break retain cycles in SizeListener callbacks that were capturing parent containers strongly.
RedwoodUIView - Used WeakReference in the sizeListener to prevent capturing valueRootView strongly.
HostProtocolAdapter - The widgetSystem was being retained even after close() was called, creating a Kotlin→Swift reference chain that kept Swift objects alive. Made widgetSystem nullable and cleared it in close() to break this chain.
Impact
These leaks affected all Treehouse-based screens in Cash iOS. After logout, UI components and their associated graphics memory (VM: Memory Tag objects) remained in memory indefinitely. With these fixes, all components are now properly deallocated when screens are dismissed.
This was tested with Receipts on the activity tab.