Purge children from view registry when UIManager is invalidated

[kkafar]

Summary:

Talking about Paper & iOS here.

In standard RN applications when a native component is removed permanently from view hierarchy it is invalidated (if it implements RCTInvalidating). Components that implement RCTInvalidating such as RNSScreenView of react-native-screens library rely on RCTInvalidating#invalidate method being called in adequate moment to release retained resources (in my case the RNSScreenView holds a strong reference to it's view controller preventing it from being garbage collected).

However in case of brownfield applications (React Native is used only for a particular view & loaded on demand, see: software-mansion/react-native-screens#1754 for discussion & app example) when view controller holding RCTRootView is dismissed and whole React Native managed view / controller tree gets deallocated, RCTInvalidating#invalidate method is not called on the dismissed components, thus in my particular use case, leading to memory leak.

Right now I've added call to RCTUIManager#_purgeChildren:fromRegistry: (which internally invalidates all components which implement RCTInvalidating) in RCTUIManager#invalidate.

Changelog:

[IOS][FIXED] - Purge children from view registry on RCTUIManager invalidation.

Test Plan:

You can run the demo provided in the issue.

Following screenshots show that memory leak in brownfield application is resolved.

Without the change (invalidate method is not being called on native components)

With the change:

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	8,843,439	-4
android	hermes	armeabi-v7a	8,152,598	+0
android	hermes	x86	9,349,234	-1
android	hermes	x86_64	9,191,954	-2
android	jsc	arm64-v8a	9,456,861	-2
android	jsc	armeabi-v7a	8,637,995	-2
android	jsc	x86	9,539,948	-2
android	jsc	x86_64	9,783,247	-1

Base commit: e64756a
Branch: main

[hoxyq]

Hey @kkafar, thanks for opening this PR! Could you please rebase and check if CI tests are passing?

Also, can someone who is more familiar with this part take a look?
cc @javache @sammy-SC

[kkafar]

Yeah, sure, done.
Waiting for CI.

[kkafar]

Hey, CI fails but it does not look like my change affects it -- rather some kind of build issue.

[javache]

Seems reasonable. I'm surprised we don't have another codepath which does this.

One other way you could get the right behaviour here is to make sure you stop each root before you tear down React Native, that will cause all views to be unmounted and also will allow useEffect destructors to run.

[javache]

Can you rebase? The pod error should've been resolved by ce39931

[facebook-github-bot]

@javache has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[kkafar]

@javache, yeah sure, rebased

[facebook-github-bot]

@javache has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[facebook-github-bot]

@javache merged this pull request in bc63e44.