From c70d7910361f56bc361ad825fe13fb2178edfeba Mon Sep 17 00:00:00 2001 From: Kudo Chien Date: Mon, 13 May 2024 04:04:07 -0700 Subject: [PATCH] Fix dangling surfaces in ReactHostImpl (#44393) Summary: Though the `ReactHost.destroy()` is not being used from OSS code, we use it at Expo for expo-dev-client to change loading apps from different dev servers. Without cleanup the `mAttachedSurfaces`, it will have dangling or duplicated attached surfaces that cause duplicated react trees. This PR tries to cleanup the `mAttachedSurfaces` from destroying. ## Changelog: [ANDROID] [FIXED] - Fixed dangling `mAttachedSurfaces` after `ReactHost.destroy()` Pull Request resolved: https://github.com/facebook/react-native/pull/44393 Test Plan: have to manually call `ReactHost.destroy()` and recreate the MainActivity without killing the process. then reload the app will startSurface for the same attached surfaces. Reviewed By: RSNara Differential Revision: D56901863 Pulled By: javache fbshipit-source-id: c7f822501d971810ac6aa7262b15da69ec41355e --- .../main/java/com/facebook/react/runtime/ReactHostImpl.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactHostImpl.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactHostImpl.java index 5ab8c2a7411965..d9406fc00b7592 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactHostImpl.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactHostImpl.java @@ -1543,9 +1543,9 @@ private Task getOrCreateDestroyTask(final String reason, @Nullable Excepti // Step 3: Stop all React Native surfaces stopAttachedSurfaces(method, reactInstance); - - // TODO(T161461674): Should we clear mAttachedSurfaces? - // Not clearing mAttachedSurfaces could lead to a memory leak. + synchronized (mAttachedSurfaces) { + mAttachedSurfaces.clear(); + } return task; },