Skip to content

Commit 4336686

Browse files
l2hyunwoot0maboro
l2hyunwoo
and
t0maboro
authored
fix(android): Fix Activity memory leak in ScreenDummyLayoutHelper due to unreleased view references and missing lifecycle cleanup (#3638)
## Description Fix Activity memory leak in `ScreenDummyLayoutHelper` caused by unreleased view references (`CoordinatorLayout`, `AppBarLayout`, `Toolbar`, `View`) when the host Activity is destroyed. The helper created dummy views using Activity-scoped `Context` to measure header heights, but never released these references on Activity destruction. This created a strong reference chain (`ScreenDummyLayoutHelper → Views → ContextThemeWrapper → Activity`) that prevents GC of destroyed Activities. Closes: #3636 ## Changes - Changed view fields from `lateinit var` to nullable `var? = null` to allow reference release - Always register `LifecycleEventListener` in constructor (regardless of init success) - Keep listener registered across Activity lifecycle (never remove it) to ensure `onHostDestroy` is always called - Release all view references, reset cache, and reset `isLayoutInitialized` in `onHostDestroy` to break the reference chain and allow re-initialization on next `onHostResume` - Wrap `computeDummyLayout` in `synchronized(this)` block with null-safe local variables, returning `0.0f` as fallback when views are already cleaned up - Synchronize `onHostDestroy` cleanup on the same monitor to prevent race conditions ## Screenshots / GIFs ### Before https://github.com/user-attachments/assets/b22c935d-9e22-4ddb-92f5-91be5caee430 ### After https://github.com/user-attachments/assets/c5946a30-3537-46a0-a15d-33183823294b ## Test plan <!-- Please name all newly added and existing test files that you tested the changes with. This section should also contain short description of steps to reproduce the issue. The reproduction code should be minimal & complete. Don't exclude exports or remove "not important" parts of reproduction example. --> 1. Launch a React Native app with native stack navigation using `react-native-screens` 2. Navigate through screens to trigger `ScreenDummyLayoutHelper` initialization 3. Trigger Activity recreation (dev reload, configuration change, or process restoration) 4. Use Android Studio Memory Profiler or LeakCanary to verify the previous Activity is no longer retained 5. Repeat to confirm no leak accumulation across multiple Activity recreations 6. Verify header height computation still works correctly after Activity recreation (headers render with correct dimensions) ## Checklist - [X] Included code example that can be used to test this change. - [X] Updated / created local changelog entries in relevant test files. - [ ] For visual changes, included screenshots / GIFs / recordings documenting the change. - [ ] For API changes, updated relevant public types. - [ ] Ensured that CI passes --------- Co-authored-by: Tomasz Boroń <tomasz.boron@swmansion.com>
1 parent a7babea commit 4336686

3 files changed

Lines changed: 197 additions & 34 deletions

File tree

‎android/src/main/java/com/swmansion/rnscreens/utils/ScreenDummyLayoutHelper.kt‎

Lines changed: 118 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
11
package com.swmansion.rnscreens.utils
22

33
import android.app.Activity
4+
import android.app.Application
45
import android.content.Context
6+
import android.os.Bundle
57
import android.util.Log
68
import android.view.View
79
import androidx.appcompat.widget.Toolbar
@@ -26,10 +28,10 @@ internal class ScreenDummyLayoutHelper(
2628
) : LifecycleEventListener {
2729
// The state required to compute header dimensions. We want this on instance rather than on class
2830
// for context access & being tied to instance lifetime.
29-
private lateinit var coordinatorLayout: CoordinatorLayout
30-
private lateinit var appBarLayout: AppBarLayout
31-
private lateinit var dummyContentView: View
32-
private lateinit var toolbar: Toolbar
31+
private var coordinatorLayout: CoordinatorLayout? = null
32+
private var appBarLayout: AppBarLayout? = null
33+
private var dummyContentView: View? = null
34+
private var toolbar: Toolbar? = null
3335
private var defaultFontSize: Float = 0f
3436
private var defaultContentInsetStartWithNavigation: Int = 0
3537

@@ -42,6 +44,9 @@ internal class ScreenDummyLayoutHelper(
4244
private var reactContextRef: WeakReference<ReactApplicationContext> =
4345
WeakReference(reactContext)
4446

47+
// We're relying on the native notification for performing cleanup, rather than relying on ReactNative `onHostDestroy`
48+
private var activityLifecycleCallbacks: Application.ActivityLifecycleCallbacks? = null
49+
4550
init {
4651
// We load the library so that we are able to communicate with our C++ code (descriptor & shadow nodes).
4752
// Basically we leak this object to C++, as its lifecycle should span throughout whole application
@@ -53,10 +58,13 @@ internal class ScreenDummyLayoutHelper(
5358
}
5459

5560
weakInstance = WeakReference(this)
56-
57-
if (!(reactContext.hasCurrentActivity() && maybeInitDummyLayoutWithHeader(reactContext))) {
58-
reactContext.addLifecycleEventListener(this)
59-
}
61+
maybeInitDummyLayoutWithHeader(reactContext)
62+
// We register as lifecycleEventListener to retry initialization in onHostResume if the
63+
// activity wasn't yet available. Once initialization succeeds, onHostResume removes this listener
64+
// from that point on cleanup is handled exclusively by ActivityLifecycleCallbacks registered on the
65+
// Application. onHostDestroy here acts only as a defensive fallback to unregister in the rare case
66+
// where init never succeeded and the lifecycleEventListener was therefore never removed.
67+
reactContext.addLifecycleEventListener(this)
6068
}
6169

6270
/**
@@ -81,7 +89,7 @@ internal class ScreenDummyLayoutHelper(
8189

8290
// We need to use activity here, as react context does not have theme attributes required by
8391
// AppBarLayout attached leading to crash.
84-
val contextWithTheme =
92+
val activity =
8593
requireNotNull(reactContext.currentActivity) {
8694
"[RNScreens] Attempt to use context detached from activity. This could happen only due to race-condition."
8795
}
@@ -91,7 +99,9 @@ internal class ScreenDummyLayoutHelper(
9199
if (isLayoutInitialized) {
92100
return true
93101
}
94-
initDummyLayoutWithHeader(contextWithTheme)
102+
initDummyLayoutWithHeader(activity)
103+
104+
registerActivityLifecycleListener(activity)
95105
}
96106
return true
97107
}
@@ -103,9 +113,9 @@ internal class ScreenDummyLayoutHelper(
103113
* to initialize the AppBarLayout.
104114
*/
105115
private fun initDummyLayoutWithHeader(contextWithTheme: Context) {
106-
coordinatorLayout = CoordinatorLayout(contextWithTheme)
116+
val newCoordinatorLayout = CoordinatorLayout(contextWithTheme)
107117

108-
appBarLayout =
118+
val newAppBarLayout =
109119
AppBarLayout(contextWithTheme).apply {
110120
layoutParams =
111121
CoordinatorLayout.LayoutParams(
@@ -114,7 +124,7 @@ internal class ScreenDummyLayoutHelper(
114124
)
115125
}
116126

117-
toolbar =
127+
val newToolbar =
118128
Toolbar(contextWithTheme).apply {
119129
title = DEFAULT_HEADER_TITLE
120130
layoutParams =
@@ -126,12 +136,16 @@ internal class ScreenDummyLayoutHelper(
126136
}
127137

128138
// We know the title text view will be there, cause we've just set title.
129-
defaultFontSize = ScreenStackHeaderConfig.findTitleTextViewInToolbar(toolbar)!!.textSize
130-
defaultContentInsetStartWithNavigation = toolbar.contentInsetStartWithNavigation
139+
val titleTextView =
140+
checkNotNull(ScreenStackHeaderConfig.findTitleTextViewInToolbar(newToolbar)) {
141+
"[RNScreens] Failed to find TextView in children of Toolbar"
142+
}
143+
defaultFontSize = titleTextView.textSize
144+
defaultContentInsetStartWithNavigation = newToolbar.contentInsetStartWithNavigation
131145

132-
appBarLayout.addView(toolbar)
146+
newAppBarLayout.addView(newToolbar)
133147

134-
dummyContentView =
148+
val newDummyContentView =
135149
View(contextWithTheme).apply {
136150
layoutParams =
137151
CoordinatorLayout.LayoutParams(
@@ -140,22 +154,32 @@ internal class ScreenDummyLayoutHelper(
140154
)
141155
}
142156

143-
coordinatorLayout.apply {
144-
addView(appBarLayout)
145-
addView(dummyContentView)
157+
newCoordinatorLayout.apply {
158+
addView(newAppBarLayout)
159+
addView(newDummyContentView)
146160
}
147161

162+
coordinatorLayout = newCoordinatorLayout
163+
appBarLayout = newAppBarLayout
164+
toolbar = newToolbar
165+
dummyContentView = newDummyContentView
166+
148167
isLayoutInitialized = true
149168
}
150169

151170
/**
152171
* Triggers layout pass on dummy view hierarchy, taking into consideration selected
153172
* ScreenStackHeaderConfig props that might have impact on final header height.
154173
*
174+
* It's called from C++ via JNI on a background thread; @Synchronized guards against concurrent `cleanUpViews`
175+
* running on the main thread (activity lifecycle), which would flip `isLayoutInitialized` and clear
176+
* the cache while a measurement is in progress.
177+
*
155178
* @param fontSize font size value as passed from JS
156179
* @return header height in dp as consumed by Yoga
157180
*/
158181
@DoNotStrip
182+
@Synchronized
159183
private fun computeDummyLayout(
160184
fontSize: Int,
161185
isTitleEmpty: Boolean,
@@ -178,7 +202,16 @@ internal class ScreenDummyLayoutHelper(
178202
return cache.headerHeight
179203
}
180204

181-
val topLevelDecorView = requireActivity().window.decorView
205+
// components below are always initialized and cleared together.
206+
val currentCoordinatorLayout = coordinatorLayout
207+
val currentAppBarLayout = appBarLayout
208+
val currentToolbar = toolbar
209+
val currentActivity = reactContextRef.get()?.currentActivity
210+
if (currentCoordinatorLayout == null || currentAppBarLayout == null || currentToolbar == null || currentActivity == null) {
211+
return 0.0f
212+
}
213+
214+
val topLevelDecorView = currentActivity.window.decorView
182215
val topInset = getDecorViewTopInset(topLevelDecorView)
183216

184217
// These dimensions are not accurate, as they do include navigation bar, however
@@ -192,42 +225,73 @@ internal class ScreenDummyLayoutHelper(
192225
View.MeasureSpec.makeMeasureSpec(decorViewHeight, View.MeasureSpec.EXACTLY)
193226

194227
if (isTitleEmpty) {
195-
toolbar.title = ""
196-
toolbar.contentInsetStartWithNavigation = 0
228+
currentToolbar.title = ""
229+
currentToolbar.contentInsetStartWithNavigation = 0
197230
} else {
198-
toolbar.title = DEFAULT_HEADER_TITLE
199-
toolbar.contentInsetStartWithNavigation = defaultContentInsetStartWithNavigation
231+
currentToolbar.title = DEFAULT_HEADER_TITLE
232+
currentToolbar.contentInsetStartWithNavigation = defaultContentInsetStartWithNavigation
200233
}
201234

202-
val textView = ScreenStackHeaderConfig.findTitleTextViewInToolbar(toolbar)
235+
val textView = ScreenStackHeaderConfig.findTitleTextViewInToolbar(currentToolbar)
203236
textView?.textSize =
204237
if (fontSize != FONT_SIZE_UNSET) fontSize.toFloat() else defaultFontSize
205238

206-
coordinatorLayout.measure(widthMeasureSpec, heightMeasureSpec)
239+
currentCoordinatorLayout.measure(widthMeasureSpec, heightMeasureSpec)
207240

208241
// It seems that measure pass would be enough, however I'm not certain whether there are no
209242
// scenarios when layout violates measured dimensions.
210-
coordinatorLayout.layout(0, 0, decorViewWidth, decorViewHeight)
243+
currentCoordinatorLayout.layout(0, 0, decorViewWidth, decorViewHeight)
211244

212245
// Include the top inset to account for the extra padding manually applied to the CustomToolbar.
213-
val totalAppBarLayoutHeight = appBarLayout.height.toFloat() + topInset
246+
val totalAppBarLayoutHeight = currentAppBarLayout.height.toFloat() + topInset
214247

215248
val headerHeight = PixelUtil.toDIPFromPixel(totalAppBarLayoutHeight)
216249
cache = CacheEntry(CacheKey(fontSize, isTitleEmpty), headerHeight)
217250
return headerHeight
218251
}
219252

253+
// We use Application.ActivityLifecycleCallbacks instead of ReactNative's LifecycleEventListener
254+
// for cleanup because it is registered on the Application object directly. We're relying on the native
255+
// lifecycle, rather than React's lifecycle.
256+
private fun registerActivityLifecycleListener(activity: Activity) {
257+
if (activityLifecycleCallbacks != null) return
258+
259+
activityLifecycleCallbacks =
260+
object : Application.ActivityLifecycleCallbacks {
261+
override fun onActivityDestroyed(destroyedActivity: Activity) {
262+
if (destroyedActivity === activity) {
263+
cleanUpViews(destroyedActivity.application)
264+
}
265+
}
266+
267+
override fun onActivityCreated(
268+
activity: Activity,
269+
savedInstanceState: Bundle?,
270+
) = Unit
271+
272+
override fun onActivityStarted(activity: Activity) = Unit
273+
274+
override fun onActivityResumed(activity: Activity) = Unit
275+
276+
override fun onActivityPaused(activity: Activity) = Unit
277+
278+
override fun onActivityStopped(activity: Activity) = Unit
279+
280+
override fun onActivitySaveInstanceState(
281+
activity: Activity,
282+
outState: Bundle,
283+
) = Unit
284+
}
285+
286+
activity.application.registerActivityLifecycleCallbacks(activityLifecycleCallbacks)
287+
}
288+
220289
private fun requireReactContext(lazyMessage: (() -> Any)? = null): ReactApplicationContext =
221290
requireNotNull(
222291
reactContextRef.get(),
223292
lazyMessage ?: { "[RNScreens] Attempt to require missing react context" },
224293
)
225294

226-
private fun requireActivity(): Activity =
227-
requireNotNull(requireReactContext().currentActivity) {
228-
"[RNScreens] Attempt to use context detached from activity"
229-
}
230-
231295
companion object {
232296
const val TAG = "ScreenDummyLayoutHelper"
233297

@@ -270,6 +334,26 @@ internal class ScreenDummyLayoutHelper(
270334
override fun onHostDestroy() {
271335
reactContextRef.get()?.removeLifecycleEventListener(this)
272336
}
337+
338+
// Called from `onActivityDestroyed` on the main thread; @Synchronized guards against concurrent
339+
// `computeDummyLayout` running on a JNI background thread, which could write a stale cache entry after cleanup.
340+
@Synchronized
341+
private fun cleanUpViews(application: Application) {
342+
coordinatorLayout = null
343+
appBarLayout = null
344+
dummyContentView = null
345+
toolbar = null
346+
347+
cache = CacheEntry.EMPTY
348+
349+
isLayoutInitialized = false
350+
351+
val callbacks = activityLifecycleCallbacks
352+
if (callbacks != null) {
353+
application.unregisterActivityLifecycleCallbacks(callbacks)
354+
activityLifecycleCallbacks = null
355+
}
356+
}
273357
}
274358

275359
private data class CacheKey(

‎apps/src/tests/issue-tests/Test3636.tsx‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
import * as React from 'react';
2+
import { Button, Text, View, StyleSheet } from 'react-native';
3+
import { NavigationContainer } from '@react-navigation/native';
4+
import {
5+
createNativeStackNavigator,
6+
NativeStackNavigationProp,
7+
} from '@react-navigation/native-stack';
8+
9+
type StackParamList = {
10+
ScreenOne: undefined;
11+
ScreenTwo: { index?: number };
12+
};
13+
14+
function ScreenOne({
15+
navigation,
16+
}: {
17+
navigation: NativeStackNavigationProp<StackParamList, 'ScreenOne'>;
18+
}) {
19+
const pushMany = () => {
20+
for (let i = 1; i <= 200; i++) {
21+
navigation.push('ScreenTwo', { index: i });
22+
}
23+
};
24+
25+
return (
26+
<View style={styles.container}>
27+
<Text style={styles.title}>ScreenOne</Text>
28+
<Button title="Push 200 ScreenTwo instances" onPress={pushMany} />
29+
</View>
30+
);
31+
}
32+
33+
function ScreenTwo({
34+
route,
35+
navigation,
36+
}: {
37+
route: any;
38+
navigation: NativeStackNavigationProp<StackParamList, 'ScreenTwo'>;
39+
}) {
40+
const index = route.params?.index ?? 0;
41+
42+
return (
43+
<View style={styles.container}>
44+
<Text style={styles.title}>ScreenTwo</Text>
45+
<Text style={styles.subtitle}>Instance Number: {index}</Text>
46+
<Button title="Pop to Top" onPress={() => navigation.popToTop()} />
47+
</View>
48+
);
49+
}
50+
51+
const Stack = createNativeStackNavigator<StackParamList>();
52+
53+
export default function App() {
54+
return (
55+
<NavigationContainer>
56+
<Stack.Navigator>
57+
<Stack.Screen name="ScreenOne" component={ScreenOne} />
58+
<Stack.Screen name="ScreenTwo" component={ScreenTwo} />
59+
</Stack.Navigator>
60+
</NavigationContainer>
61+
);
62+
}
63+
64+
const styles = StyleSheet.create({
65+
container: {
66+
flex: 1,
67+
alignItems: 'center',
68+
justifyContent: 'center',
69+
},
70+
title: {
71+
fontSize: 20,
72+
marginBottom: 10,
73+
},
74+
subtitle: {
75+
fontSize: 16,
76+
marginBottom: 10,
77+
},
78+
});

‎apps/src/tests/issue-tests/index.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,7 @@ export { default as Test3576 } from './Test3576';
181181
export { default as Test3596 } from './Test3596';
182182
export { default as Test3611 } from './Test3611';
183183
export { default as Test3617 } from './Test3617';
184+
export { default as Test3636 } from './Test3636';
184185
export { default as Test3760 } from './Test3760';
185186
export { default as Test3770 } from './Test3770';
186187
export { default as Test3793 } from './Test3793';

0 commit comments

Comments
 (0)