Skip to content

Commit

Permalink
Clean up pre-rendered surfaces properly during teardowns
Browse files Browse the repository at this point in the history
Summary:
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]

Differential Revision: D48323647

fbshipit-source-id: 7bde2696e65d520fb4d0deff8ea742e41476b9d0
  • Loading branch information
RSNara authored and facebook-github-bot committed Aug 14, 2023
1 parent 7a4e7b3 commit c95a7de
Show file tree
Hide file tree
Showing 2 changed files with 61 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
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;
Expand Down Expand Up @@ -71,6 +72,8 @@
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
Expand Down Expand Up @@ -127,6 +130,9 @@ public class ReactHostImpl implements ReactHost {
private MemoryPressureListener mMemoryPressureListener;
private @Nullable DefaultHardwareBackBtnHandler mDefaultHardwareBackBtnHandler;

private final Set<Function0<Unit>> mBeforeDestroyListeners =
Collections.synchronizedSet(new HashSet<>());

public ReactHostImpl(
Context context,
ReactHostDelegate delegate,
Expand Down Expand Up @@ -672,6 +678,20 @@ public DefaultHardwareBackBtnHandler getDefaultBackButtonHandler() {
}
}

@Override
public void addBeforeDestroyListener(@NonNull Function0<Unit> onBeforeDestroy) {
synchronized (mBeforeDestroyListeners) {
mBeforeDestroyListeners.add(onBeforeDestroy);
}
}

@Override
public void removeBeforeDestroyListener(@NonNull Function0<Unit> onBeforeDestroy) {
synchronized (mBeforeDestroyListeners) {
mBeforeDestroyListeners.remove(onBeforeDestroy);
}
}

/* package */ interface VeniceThenable<T> {
void then(T t);
}
Expand Down Expand Up @@ -1246,17 +1266,32 @@ private Task<ReactInstance> newGetOrCreateReloadTask(String reason) {
synchronized (mAttachedSurfaces) {
for (ReactSurface surface : mAttachedSurfaces) {
reactInstance.stopSurface(surface);

surface.clear();
}
}

return task;
},
mBGExecutor)
.continueWithTask(
(task) -> {
reactInstanceTaskUnwrapper.unwrap(
task, "3: Executing Before Destroy Listeners");

Set<Function0<Unit>> beforeDestroyListeners;
synchronized (mBeforeDestroyListeners) {
beforeDestroyListeners = new HashSet<>(mBeforeDestroyListeners);
}

for (Function0<Unit> 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);
Expand All @@ -1280,7 +1315,7 @@ private Task<ReactInstance> newGetOrCreateReloadTask(String reason) {
.continueWithTask(
task -> {
final ReactInstance reactInstance =
reactInstanceTaskUnwrapper.unwrap(task, "4: Destroying ReactInstance");
reactInstanceTaskUnwrapper.unwrap(task, "5: Destroying ReactInstance");

if (reactInstance == null) {
raiseSoftException(
Expand All @@ -1306,7 +1341,7 @@ private Task<ReactInstance> newGetOrCreateReloadTask(String reason) {
.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");
Expand Down Expand Up @@ -1431,7 +1466,23 @@ private Task<Void> newGetOrCreateDestroyTask(final String reason, @Nullable Exce
mBGExecutor)
.continueWithTask(
task -> {
reactInstanceTaskUnwrapper.unwrap(task, "3: Destroying ReactContext");
reactInstanceTaskUnwrapper.unwrap(
task, "3: Executing Before Destroy Listeners");

Set<Function0<Unit>> beforeDestroyListeners;
synchronized (mBeforeDestroyListeners) {
beforeDestroyListeners = new HashSet<>(mBeforeDestroyListeners);
}

for (Function0<Unit> destroyListener : beforeDestroyListeners) {
destroyListener.invoke();
}
return task;
},
mUIExecutor)
.continueWithTask(
task -> {
reactInstanceTaskUnwrapper.unwrap(task, "4: Destroying ReactContext");

final ReactContext reactContext = mBridgelessReactContextRef.getNullable();

Expand Down Expand Up @@ -1460,7 +1511,7 @@ private Task<Void> newGetOrCreateDestroyTask(final String reason, @Nullable Exce
.continueWithTask(
task -> {
final ReactInstance reactInstance =
reactInstanceTaskUnwrapper.unwrap(task, "3: Destroying ReactInstance");
reactInstanceTaskUnwrapper.unwrap(task, "5: Destroying ReactInstance");

if (reactInstance == null) {
raiseSoftException(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,4 +101,8 @@ interface ReactHost {
* @return A task that completes when React Native gets destroyed.
*/
fun destroy(reason: String, ex: Exception?): TaskInterface<Void>

fun addBeforeDestroyListener(onBeforeDestroy: () -> Unit)

fun removeBeforeDestroyListener(onBeforeDestroy: () -> Unit)
}

0 comments on commit c95a7de

Please sign in to comment.