Skip to content

Commit

Permalink
Fix dangling surfaces in ReactHostImpl (#44393)
Browse files Browse the repository at this point in the history
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.

<img src="https://github.com/facebook/react-native/assets/46429/f84d274e-aaad-4352-9e3c-6262571a5625">

This PR tries to cleanup the `mAttachedSurfaces` from destroying.

## Changelog:

[ANDROID] [FIXED] - Fixed dangling `mAttachedSurfaces` after `ReactHost.destroy()`

Pull Request resolved: #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
  • Loading branch information
Kudo authored and facebook-github-bot committed May 13, 2024
1 parent 596e14e commit c70d791
Showing 1 changed file with 3 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1543,9 +1543,9 @@ private Task<Void> 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;
},
Expand Down

0 comments on commit c70d791

Please sign in to comment.