Skip to content

Commit

Permalink
Rollout unmountApplicationOnInstanceDetach feature flag (#37512)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #37512

Rolling this out allows us to enable the asserts in Scheduler and SurfaceHandler again, which validate the correctness of our teardown routines.

Changelog: [Android][Fixed] When applications reload, the previous react root will be correctly closed

Reviewed By: NickGerleman

Differential Revision: D45905628

fbshipit-source-id: 446804c04290a799c38e4d64fb7b6be1d96e8dd4
  • Loading branch information
javache authored and facebook-github-bot committed May 24, 2023
1 parent bf1bd48 commit 3a7555f
Show file tree
Hide file tree
Showing 4 changed files with 7 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1308,11 +1308,7 @@ private void tearDownReactContext(ReactContext reactContext) {

synchronized (mAttachedReactRoots) {
for (ReactRoot reactRoot : mAttachedReactRoots) {
if (ReactFeatureFlags.unmountApplicationOnInstanceDetach) {
detachRootViewFromInstance(reactRoot, reactContext);
} else {
clearReactRoot(reactRoot);
}
detachRootViewFromInstance(reactRoot, reactContext);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -156,10 +156,4 @@ public class ReactFeatureFlags {
* HostObject pattern
*/
public static boolean useNativeState = false;

/**
* Unmount React application on ReactInstance detach. Controls rollout of change to align React
* application lifecycle with React Native instance.
*/
public static boolean unmountApplicationOnInstanceDetach = false;
}
Original file line number Diff line number Diff line change
Expand Up @@ -166,10 +166,9 @@ Scheduler::~Scheduler() {
surfaceIds.push_back(shadowTree.getSurfaceId());
});

// TODO(T88046056): Fix Android memory leak before uncommenting changes
// react_native_assert(
// surfaceIds.empty() &&
// "Scheduler was destroyed with outstanding Surfaces.");
react_native_assert(
surfaceIds.empty() &&
"Scheduler was destroyed with outstanding Surfaces.");

if (surfaceIds.empty()) {
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -325,11 +325,9 @@ void SurfaceHandler::setUIManager(UIManager const *uiManager) const noexcept {
}

SurfaceHandler::~SurfaceHandler() noexcept {
// TODO(T88046056): Fix Android memory leak before uncommenting changes
// react_native_assert(
// link_.status == Status::Unregistered &&
// "`SurfaceHandler` must be unregistered (or moved-from) before
// deallocation.");
react_native_assert(
link_.status == Status::Unregistered &&
"`SurfaceHandler` must be unregistered (or moved-from) before deallocation.");
}

} // namespace facebook::react

0 comments on commit 3a7555f

Please sign in to comment.