Skip to content

Commit 32b5fe9

Browse files
mjasikowskimjasikowski
authored
Merge pull request Expensify#72341 from software-mansion-labs/fix-leaks-in-react-native-performance
[No QA] Add patch that fixes memory leaks in `react-native-performance` on Android
2 parents 27845f8 + 1af457d commit 32b5fe9

File tree

2 files changed

+215
-0
lines changed

2 files changed

+215
-0
lines changed

‎patches/react-native-performance/details.md‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,3 +16,20 @@
1616
- Upstream PR/issue: https://github.com/oblador/react-native-performance/pull/117
1717
- E/App issue: https://github.com/Expensify/App/issues/66231
1818
- PR introducing patch: https://github.com/Expensify/App/pull/66230
19+
20+
### [react-native-performance+5.1.4+002+fix-listeners-memory-leak.patch](react-native-performance+5.1.4+002+fix-listeners-memory-leak.patch)
21+
22+
- Reason:
23+
24+
```
25+
`react-native-performance` has a memory leak where ReactMarker listeners are not properly removed when the module is destroyed.
26+
Additionally, `setupListener()` can be called multiple times when `getPackages()` is called multiple times, causing duplicate listeners.
27+
This patch:
28+
1. Extracts listeners to class fields to avoid duplication and enable later removal
29+
2. Properly removes listeners in the `invalidate()` method
30+
3. Fixes the logic in `removeListener()` to only remove if the listener exists
31+
```
32+
33+
- Upstream PR/issue: https://github.com/oblador/react-native-performance/pull/118
34+
- E/App issue: https://github.com/Expensify/App/issues/72343
35+
- PR introducing patch: https://github.com/Expensify/App/pull/72341

‎patches/react-native-performance/react-native-performance+5.1.4+002+fix-listeners-memory-leak.patch‎

Lines changed: 198 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,198 @@
1+
diff --git a/node_modules/react-native-performance/android/src/main/java/com/oblador/performance/PerformanceModule.java b/node_modules/react-native-performance/android/src/main/java/com/oblador/performance/PerformanceModule.java
2+
index 66c94ee..131c88b 100644
3+
--- a/node_modules/react-native-performance/android/src/main/java/com/oblador/performance/PerformanceModule.java
4+
+++ b/node_modules/react-native-performance/android/src/main/java/com/oblador/performance/PerformanceModule.java
5+
@@ -25,12 +25,74 @@ public class PerformanceModule extends ReactContextBaseJavaModule implements Tur
6+
private static final Queue<PerformanceEntry> markBuffer = new ConcurrentLinkedQueue<>();
7+
private static boolean didEmit = false;
8+
9+
+ private static final ReactMarker.MarkerListener startupMarkerListener = (name, tag, instanceKey) -> {
10+
+ switch (name) {
11+
+ case RELOAD:
12+
+ clearMarkBuffer();
13+
+ addMark(new PerformanceMark(BRIDGE_SETUP_START, SystemClock.uptimeMillis()));
14+
+ break;
15+
+ case ATTACH_MEASURED_ROOT_VIEWS_END:
16+
+ case ATTACH_MEASURED_ROOT_VIEWS_START:
17+
+ case BUILD_NATIVE_MODULE_REGISTRY_END:
18+
+ case BUILD_NATIVE_MODULE_REGISTRY_START:
19+
+ case CONTENT_APPEARED:
20+
+ case CREATE_CATALYST_INSTANCE_END:
21+
+ case CREATE_CATALYST_INSTANCE_START:
22+
+ case CREATE_REACT_CONTEXT_END:
23+
+ case CREATE_REACT_CONTEXT_START:
24+
+ case CREATE_UI_MANAGER_MODULE_CONSTANTS_END:
25+
+ case CREATE_UI_MANAGER_MODULE_CONSTANTS_START:
26+
+ case CREATE_UI_MANAGER_MODULE_END:
27+
+ case CREATE_UI_MANAGER_MODULE_START:
28+
+ case CREATE_VIEW_MANAGERS_END:
29+
+ case CREATE_VIEW_MANAGERS_START:
30+
+ case DOWNLOAD_END:
31+
+ case DOWNLOAD_START:
32+
+ case LOAD_REACT_NATIVE_SO_FILE_END:
33+
+ case LOAD_REACT_NATIVE_SO_FILE_START:
34+
+ case PRE_RUN_JS_BUNDLE_START:
35+
+ case PRE_SETUP_REACT_CONTEXT_END:
36+
+ case PRE_SETUP_REACT_CONTEXT_START:
37+
+ case PROCESS_CORE_REACT_PACKAGE_END:
38+
+ case PROCESS_CORE_REACT_PACKAGE_START:
39+
+ case REACT_CONTEXT_THREAD_END:
40+
+ case REACT_CONTEXT_THREAD_START:
41+
+ case RUN_JS_BUNDLE_END:
42+
+ case RUN_JS_BUNDLE_START:
43+
+ case SETUP_REACT_CONTEXT_END:
44+
+ case SETUP_REACT_CONTEXT_START:
45+
+ case VM_INIT:
46+
+ long startTime = SystemClock.uptimeMillis();
47+
+ addMark(new PerformanceMark(getMarkName(name), startTime));
48+
+ break;
49+
+ }
50+
+ };
51+
+
52+
+ private final ReactMarker.MarkerListener contentAppearedListener = (name, tag, instanceKey) -> {
53+
+ switch (name) {
54+
+ case CONTENT_APPEARED:
55+
+ eventsBuffered = false;
56+
+ emitNativeStartupTime();
57+
+ emitBufferedMarks();
58+
+ break;
59+
+ case RELOAD:
60+
+ eventsBuffered = true;
61+
+ break;
62+
+ }
63+
+ };
64+
+
65+
public PerformanceModule(@NonNull final ReactApplicationContext reactContext) {
66+
super(reactContext);
67+
setupMarkerListener();
68+
setupNativeMarkerListener();
69+
}
70+
71+
+ private void setupMarkerListener() {
72+
+ ReactMarker.addListener(
73+
+ contentAppearedListener
74+
+ );
75+
+ }
76+
+
77+
private void setupNativeMarkerListener() {
78+
RNPerformance.getInstance().addListener(this);
79+
}
80+
@@ -38,51 +100,7 @@ public class PerformanceModule extends ReactContextBaseJavaModule implements Tur
81+
// Need to set up the marker listener before the react module is initialized
82+
// to capture all events
83+
public static void setupListener() {
84+
- ReactMarker.addListener(
85+
- (name, tag, instanceKey) -> {
86+
- switch (name) {
87+
- case RELOAD:
88+
- clearMarkBuffer();
89+
- addMark(new PerformanceMark(BRIDGE_SETUP_START, SystemClock.uptimeMillis()));
90+
- break;
91+
- case ATTACH_MEASURED_ROOT_VIEWS_END:
92+
- case ATTACH_MEASURED_ROOT_VIEWS_START:
93+
- case BUILD_NATIVE_MODULE_REGISTRY_END:
94+
- case BUILD_NATIVE_MODULE_REGISTRY_START:
95+
- case CONTENT_APPEARED:
96+
- case CREATE_CATALYST_INSTANCE_END:
97+
- case CREATE_CATALYST_INSTANCE_START:
98+
- case CREATE_REACT_CONTEXT_END:
99+
- case CREATE_REACT_CONTEXT_START:
100+
- case CREATE_UI_MANAGER_MODULE_CONSTANTS_END:
101+
- case CREATE_UI_MANAGER_MODULE_CONSTANTS_START:
102+
- case CREATE_UI_MANAGER_MODULE_END:
103+
- case CREATE_UI_MANAGER_MODULE_START:
104+
- case CREATE_VIEW_MANAGERS_END:
105+
- case CREATE_VIEW_MANAGERS_START:
106+
- case DOWNLOAD_END:
107+
- case DOWNLOAD_START:
108+
- case LOAD_REACT_NATIVE_SO_FILE_END:
109+
- case LOAD_REACT_NATIVE_SO_FILE_START:
110+
- case PRE_RUN_JS_BUNDLE_START:
111+
- case PRE_SETUP_REACT_CONTEXT_END:
112+
- case PRE_SETUP_REACT_CONTEXT_START:
113+
- case PROCESS_CORE_REACT_PACKAGE_END:
114+
- case PROCESS_CORE_REACT_PACKAGE_START:
115+
- case REACT_CONTEXT_THREAD_END:
116+
- case REACT_CONTEXT_THREAD_START:
117+
- case RUN_JS_BUNDLE_END:
118+
- case RUN_JS_BUNDLE_START:
119+
- case SETUP_REACT_CONTEXT_END:
120+
- case SETUP_REACT_CONTEXT_START:
121+
- case VM_INIT:
122+
- long startTime = SystemClock.uptimeMillis();
123+
- addMark(new PerformanceMark(getMarkName(name), startTime));
124+
- break;
125+
-
126+
- }
127+
- }
128+
- );
129+
+ ReactMarker.addListener(startupMarkerListener);
130+
}
131+
132+
private static void clearMarkBuffer() {
133+
@@ -118,27 +136,19 @@ public class PerformanceModule extends ReactContextBaseJavaModule implements Tur
134+
return PERFORMANCE_MODULE;
135+
}
136+
137+
+ public void addListener(String eventName) {
138+
+
139+
+ }
140+
+
141+
+ public void removeListeners(double count) {
142+
+
143+
+ }
144+
+
145+
private void emitNativeStartupTime() {
146+
safelyEmitMark(new PerformanceMark("nativeLaunchStart", StartTimeProvider.getStartTime()));
147+
safelyEmitMark(new PerformanceMark("nativeLaunchEnd", StartTimeProvider.getEndTime()));
148+
}
149+
150+
- private void setupMarkerListener() {
151+
- ReactMarker.addListener(
152+
- (name, tag, instanceKey) -> {
153+
- switch (name) {
154+
- case CONTENT_APPEARED:
155+
- eventsBuffered = false;
156+
- emitNativeStartupTime();
157+
- emitBufferedMarks();
158+
- break;
159+
- case RELOAD:
160+
- eventsBuffered = true;
161+
- break;
162+
- }
163+
- }
164+
- );
165+
- }
166+
167+
private void safelyEmitMark(PerformanceEntry entry) {
168+
if (eventsBuffered) {
169+
@@ -217,13 +229,9 @@ public class PerformanceModule extends ReactContextBaseJavaModule implements Tur
170+
}
171+
172+
@Override
173+
- public void onCatalystInstanceDestroy() {
174+
- super.onCatalystInstanceDestroy();
175+
+ public void invalidate() {
176+
+ super.invalidate();
177+
RNPerformance.getInstance().removeListener(this);
178+
- }
179+
-
180+
- // Fix new arch runtime error
181+
- public void addListener(String eventName) {
182+
-
183+
+ ReactMarker.removeListener(contentAppearedListener);
184+
}
185+
}
186+
diff --git a/node_modules/react-native-performance/android/src/main/java/com/oblador/performance/RNPerformance.java b/node_modules/react-native-performance/android/src/main/java/com/oblador/performance/RNPerformance.java
187+
index 4d6223b..2f6643b 100644
188+
--- a/node_modules/react-native-performance/android/src/main/java/com/oblador/performance/RNPerformance.java
189+
+++ b/node_modules/react-native-performance/android/src/main/java/com/oblador/performance/RNPerformance.java
190+
@@ -43,7 +43,7 @@ public class RNPerformance {
191+
192+
@DoNotStrip
193+
protected void removeListener(MarkerListener listener) {
194+
- if (!sListeners.contains(listener)) {
195+
+ if (sListeners.contains(listener)) {
196+
sListeners.remove(listener);
197+
}
198+
}

0 commit comments

Comments
 (0)