fix(iOS): Fix memory leaks in RNSScreen for brownfield environments#3459
fix(iOS): Fix memory leaks in RNSScreen for brownfield environments#3459intmain wants to merge 1 commit intosoftware-mansion:mainfrom
Conversation
This commit fixes critical memory leaks in RNSScreen that occur when React Native is completely dismissed in brownfield applications, preventing RNSScreen and Hermes from being deallocated. Root causes addressed: 1. Retain cycle between RNSScreen and RNSScreenView - Changed controller property from strong to weak reference - Added _retainedController to temporarily hold strong reference until parent takes ownership 2. Retain cycle with CADisplayLink in setupProgressNotification - Implemented RNSWeakProxy pattern to break the strong reference cycle - CADisplayLink now holds weak reference to RNSScreen through proxy 3. Missing cleanup of _animationTimer - Added invalidation at the start of setupProgressNotification (handles multiple viewWillDisappear calls) - Added dealloc method to RNSScreen to clean up _animationTimer - Added nil assignment after invalidation in completion block 4. Proper lifetime management - Release _retainedController in willMoveToParentViewController when parent takes ownership Tested in brownfield environment with complete RN teardown - verified all memory leaks resolved.
kkafar
left a comment
There was a problem hiding this comment.
Hey thanks for the PR.
This looks similar to the problem a solved years back on Paper: facebook/react-native#38617 & I think it requires similar solution - a fix in core to invalidate the components.
Therefore I'll treat this PR more like an issue report and seek solution in core. We'll look into this in upcoming weeks.
Thanks.
|
We've encountered this issue in our brownfield integration, yet for us it seems that it was RNSScreenStack not invalidating screens. Our current fix is adding The screens seems to be properly discarded after (since invalidate effectively removes cycle) and I feel invalidate should be there - similar to avoiding cycle by using weak references as in this PR |
Description
This PR fixes critical memory leaks in
RNSScreenthat occur when React Native is completely dismissed in brownfield applications. In brownfield environments, when RN views and the Hermes engine are fully torn down,RNSScreenand related objects were not being deallocated, causing significant memory leaks.Root causes identified:
deallocmethod and incomplete timer cleanup insetupProgressNotificationFixes memory leaks in brownfield React Native environments.
Changes
RNSScreenView.controllerproperty fromstrongtoweakreference_retainedControllerinternal property to temporarily hold strong reference until parentViewController takes ownershipRNSWeakProxyclass using NSProxy pattern to break CADisplayLink retain cyclesetupProgressNotificationto handle multipleviewWillDisappearcallsdeallocmethod toRNSScreento properly clean up_animationTimerinvalidatein completion block_retainedControllerinwillMoveToParentViewControllerwhen parent takes ownershipTest code and steps to reproduce
Testing for memory leaks in brownfield environment:
Prerequisites: Brownfield iOS app with React Native integration
Steps:
Verification:
You can add a temporary log to verify dealloc is called:
When you completely dismiss RN, you should see the log message in console.
Expected behavior:
Testing in FabricExample:
Checklist
Note: This is an iOS-only internal implementation fix with no API changes, so TypeScript types and documentation updates are not required.