Skip to content

Commit ddbb31b

Browse files
zeyapfacebook-github-bot
authored andcommitted
Clear AnimatedPropsRegistry on surface stop
Summary: ## Changelog: [Internal] [Fixed] - Clear AnimatedPropsRegistry on surface stop When `useSharedAnimatedBackend` is enabled, the `AnimatedPropsRegistry` accumulates `SurfaceContext` entries (containing `shared_ptr<ShadowNodeFamily>` and `PropsSnapshot` data) for each surface that has animated views. These entries are never cleaned up when a surface is destroyed via `UIManager::stopSurface()`, because that method only calls the legacy `stopSurfaceForAnimationDelegate()` — the shared backend's registry is untouched. We also see increased `RetryableMountingLayerException` errors in production which stack trace shows there are mount items committing to surface that no longer exists. This change: 1. Adds `animationBackend_->clearRegistry(surfaceId)` to `UIManager::stopSurface()` 2. Changes `AnimatedPropsRegistry::clear()` to fully erase the `surfaceContexts_` map entry instead of just clearing its contents Differential Revision: D101354994
1 parent 77d3df8 commit ddbb31b

4 files changed

Lines changed: 237 additions & 5 deletions

File tree

packages/react-native/ReactCommon/react/renderer/animationbackend/AnimatedPropsRegistry.cpp

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -93,10 +93,7 @@ AnimatedPropsRegistry::getMap(SurfaceId surfaceId) {
9393

9494
void AnimatedPropsRegistry::clear(SurfaceId surfaceId) {
9595
auto lock = std::lock_guard(mutex_);
96-
97-
auto& surfaceContext = surfaceContexts_[surfaceId];
98-
surfaceContext.families.clear();
99-
surfaceContext.map.clear();
96+
surfaceContexts_.erase(surfaceId);
10097
}
10198

10299
} // namespace facebook::react

packages/react-native/ReactCommon/react/renderer/uimanager/UIManager.cpp

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -272,9 +272,13 @@ void UIManager::setSurfaceProps(
272272
ShadowTree::Unique UIManager::stopSurface(SurfaceId surfaceId) const {
273273
TraceSection s("UIManager::stopSurface");
274274

275-
// Stop any ongoing animations.
275+
// Stop any ongoing layout animations.
276276
stopSurfaceForAnimationDelegate(surfaceId);
277277

278+
if (ReactNativeFeatureFlags::useSharedAnimatedBackend()) {
279+
animationBackend_->clearRegistry(surfaceId);
280+
}
281+
278282
// Waiting for all concurrent commits to be finished and unregistering the
279283
// `ShadowTree`.
280284
auto shadowTree = getShadowTreeRegistry().remove(surfaceId);

packages/rn-tester/js/examples/Animated/AnimatedIndex.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
import type {RNTesterModule} from '../../types/RNTesterTypes';
1212

13+
import AnimatedMemoryLeakExample from './AnimatedMemoryLeakExample';
1314
import ColorStylesExample from './ColorStylesExample';
1415
import CombineExample from './CombineExample';
1516
import ComposeAnimationsWithEasingExample from './ComposeAnimationsWithEasingExample';
@@ -35,6 +36,7 @@ export default {
3536
'build and maintain.',
3637
showIndividualExamples: true,
3738
examples: [
39+
AnimatedMemoryLeakExample,
3840
TransformStylesExample,
3941
ColorStylesExample,
4042
FadeInViewExample,
Lines changed: 229 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,229 @@
1+
/**
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*
7+
* @flow strict-local
8+
* @format
9+
*/
10+
11+
import type {RNTesterModuleExample} from '../../types/RNTesterTypes';
12+
13+
import RNTesterButton from '../../components/RNTesterButton';
14+
import * as React from 'react';
15+
import {useCallback, useEffect, useRef, useState} from 'react';
16+
import {Animated, ScrollView, StyleSheet, Text, View} from 'react-native';
17+
18+
/**
19+
* A single animated item that fades in and pulses opacity.
20+
* Each instance creates animated nodes and connects them to a view
21+
* via the native animated driver.
22+
*/
23+
function AnimatedItem({id}: {id: number}) {
24+
const opacity = useRef(new Animated.Value(0)).current;
25+
26+
useEffect(() => {
27+
// Fade in then loop a pulse animation
28+
Animated.sequence([
29+
Animated.timing(opacity, {
30+
toValue: 1,
31+
duration: 300,
32+
useNativeDriver: true,
33+
}),
34+
Animated.loop(
35+
Animated.sequence([
36+
Animated.timing(opacity, {
37+
toValue: 0.3,
38+
duration: 1000,
39+
useNativeDriver: true,
40+
}),
41+
Animated.timing(opacity, {
42+
toValue: 1,
43+
duration: 1000,
44+
useNativeDriver: true,
45+
}),
46+
]),
47+
),
48+
]).start();
49+
50+
return () => {
51+
opacity.stopAnimation();
52+
};
53+
}, [opacity]);
54+
55+
return (
56+
<Animated.View style={[styles.item, {opacity}]}>
57+
<Text style={styles.itemText}>Item #{id}</Text>
58+
</Animated.View>
59+
);
60+
}
61+
62+
/**
63+
* A "page" of animated items. Each page simulates a surface with many
64+
* animated views (like a feed screen with animated like buttons, images, etc.)
65+
*/
66+
function AnimatedPage({pageId, itemCount}: {pageId: number, itemCount: number}) {
67+
const items = [];
68+
for (let i = 0; i < itemCount; i++) {
69+
items.push(<AnimatedItem key={`${pageId}-${i}`} id={pageId * 1000 + i} />);
70+
}
71+
return (
72+
<View style={styles.page}>
73+
<Text style={styles.pageTitle}>
74+
Page {pageId} ({itemCount} animated views)
75+
</Text>
76+
<ScrollView style={styles.scrollView}>{items}</ScrollView>
77+
</View>
78+
);
79+
}
80+
81+
/**
82+
* Memory Leak Reproduction:
83+
*
84+
* This example simulates the pattern that causes the AnimatedPropsRegistry
85+
* memory leak. It creates and destroys "pages" of animated views, mimicking
86+
* navigation between screens in FB4A.
87+
*
88+
* When useSharedAnimatedBackend is enabled, each page's animated views get
89+
* entries in the AnimatedPropsRegistry (shared_ptr<ShadowNodeFamily> +
90+
* PropsSnapshot). When the page is unmounted, these entries should be cleaned
91+
* up via clearRegistry() in UIManager::stopSurface().
92+
*
93+
* To test:
94+
* 1. Open Android Studio profiler and attach to the Catalyst app
95+
* 2. Click "Auto-cycle pages" to start creating/destroying animated pages
96+
* 3. Watch native memory (C++ heap) in the profiler
97+
* 4. With the leak: native memory grows monotonically
98+
* 5. With the fix (D101354994): native memory stays stable
99+
*
100+
* You can also use "Add 50 items" to increase animated view count per page,
101+
* then cycle to amplify the leak.
102+
*/
103+
function AnimatedMemoryLeakRepro() {
104+
const [currentPage, setCurrentPage] = useState(0);
105+
const [itemsPerPage, setItemsPerPage] = useState(20);
106+
const [totalPagesCreated, setTotalPagesCreated] = useState(0);
107+
const [autoCycling, setAutoCycling] = useState(false);
108+
const intervalRef = useRef<ReturnType<typeof setInterval> | null>(null);
109+
110+
const nextPage = useCallback(() => {
111+
setCurrentPage(prev => prev + 1);
112+
setTotalPagesCreated(prev => prev + 1);
113+
}, []);
114+
115+
const toggleAutoCycle = useCallback(() => {
116+
if (autoCycling) {
117+
if (intervalRef.current) {
118+
clearInterval(intervalRef.current);
119+
intervalRef.current = null;
120+
}
121+
setAutoCycling(false);
122+
} else {
123+
intervalRef.current = setInterval(() => {
124+
setCurrentPage(prev => prev + 1);
125+
setTotalPagesCreated(prev => prev + 1);
126+
}, 2000); // New page every 2 seconds
127+
setAutoCycling(true);
128+
}
129+
}, [autoCycling]);
130+
131+
useEffect(() => {
132+
return () => {
133+
if (intervalRef.current) {
134+
clearInterval(intervalRef.current);
135+
}
136+
};
137+
}, []);
138+
139+
return (
140+
<View style={styles.container}>
141+
<View style={styles.controls}>
142+
<Text style={styles.stats}>
143+
Current page: {currentPage} | Total created: {totalPagesCreated} |
144+
Items/page: {itemsPerPage}
145+
</Text>
146+
<View style={styles.buttonRow}>
147+
<RNTesterButton onPress={nextPage}>
148+
Next Page
149+
</RNTesterButton>
150+
<RNTesterButton onPress={toggleAutoCycle}>
151+
{autoCycling ? 'Stop auto-cycle' : 'Auto-cycle pages (2s)'}
152+
</RNTesterButton>
153+
</View>
154+
<View style={styles.buttonRow}>
155+
<RNTesterButton onPress={() => setItemsPerPage(prev => prev + 50)}>
156+
Add 50 items/page
157+
</RNTesterButton>
158+
<RNTesterButton
159+
onPress={() => setItemsPerPage(prev => Math.max(5, prev - 50))}>
160+
Remove 50 items/page
161+
</RNTesterButton>
162+
</View>
163+
</View>
164+
{/* Key forces full unmount/remount on page change */}
165+
<AnimatedPage
166+
key={`page-${currentPage}`}
167+
pageId={currentPage}
168+
itemCount={itemsPerPage}
169+
/>
170+
</View>
171+
);
172+
}
173+
174+
const styles = StyleSheet.create({
175+
container: {
176+
flex: 1,
177+
},
178+
controls: {
179+
padding: 10,
180+
backgroundColor: '#f0f0f0',
181+
borderBottomWidth: 1,
182+
borderBottomColor: '#ccc',
183+
},
184+
stats: {
185+
fontSize: 12,
186+
marginBottom: 8,
187+
fontFamily: 'monospace',
188+
},
189+
buttonRow: {
190+
flexDirection: 'row',
191+
justifyContent: 'space-around',
192+
marginBottom: 4,
193+
},
194+
page: {
195+
flex: 1,
196+
},
197+
pageTitle: {
198+
fontSize: 16,
199+
fontWeight: 'bold',
200+
padding: 10,
201+
backgroundColor: '#e0e0ff',
202+
},
203+
scrollView: {
204+
flex: 1,
205+
},
206+
item: {
207+
backgroundColor: 'deepskyblue',
208+
borderWidth: 1,
209+
borderColor: 'dodgerblue',
210+
padding: 12,
211+
marginHorizontal: 10,
212+
marginVertical: 4,
213+
borderRadius: 8,
214+
},
215+
itemText: {
216+
color: 'white',
217+
fontWeight: '600',
218+
},
219+
});
220+
221+
export default ({
222+
title: 'Memory Leak Repro (AnimatedPropsRegistry)',
223+
name: 'animatedMemoryLeak',
224+
description:
225+
'Simulates surface transitions with animated views to reproduce ' +
226+
'AnimatedPropsRegistry memory leak. Use with profiler to measure native ' +
227+
'memory growth. Auto-cycle creates/destroys pages every 2s.',
228+
render: (): React.Node => <AnimatedMemoryLeakRepro />,
229+
}: RNTesterModuleExample);

0 commit comments

Comments
 (0)