From 80b665966fc5dffbe950f7ba4392d97d4a20d514 Mon Sep 17 00:00:00 2001 From: Ramanpreet Nara Date: Mon, 21 Aug 2023 12:43:42 -0700 Subject: [PATCH] Clean up pre-rendered surfaces properly during teardowns (#39000) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/39000 Whenever React Native tears down (including on logout), we need to drop unconsumed Pre-rendering surfaces. D45012714 initially implemented this change, but this diff wasn't complete: it would only drop unconsumed pre-rendered surfaces when ***the Facebook infra* initiated** React Native to tear down. But, React Native could initiate tear down **by iteself** (e.g: via an uncaught exception on the Native Modules thread). ## Changes In this diff, make the React Manager support an onBeforeDestroy listener. Then, integrate these listeners into the teardown/reload algorithms. That way, no matter how React Native tears down, we **alwasy** drop unconsumed pre-rendered surfaces. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D48323647 fbshipit-source-id: 28f99500eea457b0e3c420fed28c434f8794ac23 --- .../react/bridgeless/ReactHostImpl.java | 62 +++++++++++++++++-- .../facebook/react/interfaces/ReactHost.kt | 4 ++ 2 files changed, 61 insertions(+), 5 deletions(-) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridgeless/ReactHostImpl.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridgeless/ReactHostImpl.java index 4ca4441297a..3ef32a03be4 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridgeless/ReactHostImpl.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridgeless/ReactHostImpl.java @@ -16,6 +16,7 @@ import static java.lang.Boolean.TRUE; import android.app.Activity; import android.content.Context; import android.os.Bundle; +import androidx.annotation.NonNull; import androidx.annotation.Nullable; import com.facebook.common.logging.FLog; import com.facebook.infer.annotation.Assertions; @@ -71,6 +72,8 @@ import java.util.concurrent.Executor; import java.util.concurrent.Executors; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; +import kotlin.Unit; +import kotlin.jvm.functions.Function0; /** * A ReactHost is an object that manages a single {@link ReactInstance}. A ReactHost can be @@ -128,6 +131,9 @@ public class ReactHostImpl implements ReactHost { private MemoryPressureListener mMemoryPressureListener; private @Nullable DefaultHardwareBackBtnHandler mDefaultHardwareBackBtnHandler; + private final Set> mBeforeDestroyListeners = + Collections.synchronizedSet(new HashSet<>()); + public ReactHostImpl( Context context, ReactHostDelegate delegate, @@ -677,6 +683,20 @@ public class ReactHostImpl implements ReactHost { } } + @Override + public void addBeforeDestroyListener(@NonNull Function0 onBeforeDestroy) { + synchronized (mBeforeDestroyListeners) { + mBeforeDestroyListeners.add(onBeforeDestroy); + } + } + + @Override + public void removeBeforeDestroyListener(@NonNull Function0 onBeforeDestroy) { + synchronized (mBeforeDestroyListeners) { + mBeforeDestroyListeners.remove(onBeforeDestroy); + } + } + /* package */ interface VeniceThenable { void then(T t); } @@ -1294,9 +1314,25 @@ public class ReactHostImpl implements ReactHost { return task; }, mBGExecutor) + .continueWithTask( + (task) -> { + reactInstanceTaskUnwrapper.unwrap( + task, "3: Executing Before Destroy Listeners"); + + Set> beforeDestroyListeners; + synchronized (mBeforeDestroyListeners) { + beforeDestroyListeners = new HashSet<>(mBeforeDestroyListeners); + } + + for (Function0 destroyListener : beforeDestroyListeners) { + destroyListener.invoke(); + } + return task; + }, + mUIExecutor) .continueWithTask( task -> { - reactInstanceTaskUnwrapper.unwrap(task, "3: Destroying ReactContext"); + reactInstanceTaskUnwrapper.unwrap(task, "4: Destroying ReactContext"); log(method, "Removing memory pressure listener"); mMemoryPressureRouter.removeMemoryPressureListener(mMemoryPressureListener); @@ -1320,7 +1356,7 @@ public class ReactHostImpl implements ReactHost { .continueWithTask( task -> { final ReactInstance reactInstance = - reactInstanceTaskUnwrapper.unwrap(task, "4: Destroying ReactInstance"); + reactInstanceTaskUnwrapper.unwrap(task, "5: Destroying ReactInstance"); if (reactInstance == null) { raiseSoftException( @@ -1346,7 +1382,7 @@ public class ReactHostImpl implements ReactHost { .continueWithTask( task -> { final ReactInstance reactInstance = - reactInstanceTaskUnwrapper.unwrap(task, "5: Restarting surfaces"); + reactInstanceTaskUnwrapper.unwrap(task, "7: Restarting surfaces"); if (reactInstance == null) { raiseSoftException(method, "Skipping surface restart: ReactInstance null"); @@ -1463,7 +1499,23 @@ public class ReactHostImpl implements ReactHost { mBGExecutor) .continueWithTask( task -> { - reactInstanceTaskUnwrapper.unwrap(task, "3: Destroying ReactContext"); + reactInstanceTaskUnwrapper.unwrap( + task, "3: Executing Before Destroy Listeners"); + + Set> beforeDestroyListeners; + synchronized (mBeforeDestroyListeners) { + beforeDestroyListeners = new HashSet<>(mBeforeDestroyListeners); + } + + for (Function0 destroyListener : beforeDestroyListeners) { + destroyListener.invoke(); + } + return task; + }, + mUIExecutor) + .continueWithTask( + task -> { + reactInstanceTaskUnwrapper.unwrap(task, "4: Destroying ReactContext"); final ReactContext reactContext = mBridgelessReactContextRef.getNullable(); @@ -1492,7 +1544,7 @@ public class ReactHostImpl implements ReactHost { .continueWithTask( task -> { final ReactInstance reactInstance = - reactInstanceTaskUnwrapper.unwrap(task, "3: Destroying ReactInstance"); + reactInstanceTaskUnwrapper.unwrap(task, "5: Destroying ReactInstance"); if (reactInstance == null) { raiseSoftException( diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/interfaces/ReactHost.kt b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/interfaces/ReactHost.kt index 74da0ae58f5..4c5ef17ae36 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/interfaces/ReactHost.kt +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/interfaces/ReactHost.kt @@ -101,4 +101,8 @@ interface ReactHost { * @return A task that completes when React Native gets destroyed. */ fun destroy(reason: String, ex: Exception?): TaskInterface + + fun addBeforeDestroyListener(onBeforeDestroy: () -> Unit) + + fun removeBeforeDestroyListener(onBeforeDestroy: () -> Unit) }