Skip to content

Commit

Permalink
Migrate ReactHost / ReactInstanceManager destroy() call sites to use …
Browse files Browse the repository at this point in the history
…invalidate() (#45082)

Summary:
Pull Request resolved: #45082

Changelog: [Android][Breaking] `ReactNativeHost` invalidates the instance manager on `clear()`

Changes `ReactNativeHost.clear()` to invalidate the underlying `ReactInstanceManager`, rather than merely destroying the instance.

This is technically a **breaking change** because the underlying `ReactInstanceManager` may have escaped (via `ReactNativeHost.getReactInstanceManager()`) before the `clear()` call. In my reading of the API and of usages like [this one in Expo](https://github.com/expo/expo/blob/23a905b17065703882ebeda1fc9f65a05cc69fa7/packages/expo-dev-menu-interface/android/src/main/java/expo/interfaces/devmenu/ReactHostWrapper.kt#L117), this should rarely occur in practice.

The plan:
1. D58811090: Add the basic `invalidate()` functionality.
2. **[This diff]**: Add `invalidate()` call sites where it makes sense in core.
3. [Upcoming diff]: Keep the Fusebox debugging target registered until the Host is explicitly invalidated.

Reviewed By: javache

Differential Revision: D58811091
  • Loading branch information
motiz88 authored and facebook-github-bot committed Jun 24, 2024
1 parent 942db9e commit 1afa646
Showing 1 changed file with 10 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,13 @@ protected ReactNativeHost(Application application) {
mApplication = application;
}

/** Get the current {@link ReactInstanceManager} instance, or create one. */
/**
* Get the current {@link ReactInstanceManager} instance, or create one.
*
* <p>NOTE: Care must be taken when storing this reference outside of the ReactNativeHost
* lifecycle. The ReactInstanceManager will be invalidated during {@link #clear()}, and may not be
* used again afterwards.
*/
public ReactInstanceManager getReactInstanceManager() {
if (mReactInstanceManager == null) {
ReactMarker.logMarker(ReactMarkerConstants.INIT_REACT_RUNTIME_START);
Expand All @@ -64,11 +70,12 @@ public boolean hasInstance() {
}

/**
* Destroy the current instance and release the internal reference to it, allowing it to be GCed.
* Destroy the current instance and invalidate the internal ReactInstanceManager, reclaiming its
* resources and preventing it from being reused.
*/
public void clear() {
if (mReactInstanceManager != null) {
mReactInstanceManager.destroy();
mReactInstanceManager.invalidate();
mReactInstanceManager = null;
}
}
Expand Down

0 comments on commit 1afa646

Please sign in to comment.