Fix race between teardown and PreAllocateView/CreateView MountItem creation

Summary:
Commits can happen during navigation/teardown which creates mount items. If we throw an exception during
teardown because we expect the Context to still be around, we crash too often. Instead, I will rely on
logic in FabricUIManager to ignore queued MountItems if we try to execute them after the surface has been torn down;
and we move the IllegalStateException to actual execution of the mount item in case there's an edge-case we're missing.

Changelog: [Internal]

Reviewed By: mdvacca

Differential Revision: D22470102

fbshipit-source-id: ad98c03994969a3c3f300d6551e90b6516ed2d8b
This commit is contained in:
Joshua Gross
2020-07-09 20:50:00 -07:00
committed by Facebook GitHub Bot
parent e549f6984e
commit 3503d722a1
3 changed files with 27 additions and 7 deletions
@@ -314,7 +314,11 @@ public class FabricUIManager implements UIManager, LifecycleEventListener {
@Nullable ReadableMap props,
@Nullable Object stateWrapper,
boolean isLayoutable) {
ThemedReactContext context = mReactContextForRootTag.get(rootTag);
// This could be null if teardown/navigation away from a surface on the main thread happens
// while a commit is being processed in a different thread. By contract we expect this to be
// possible at teardown, but this race should *never* happen at startup.
@Nullable ThemedReactContext context = mReactContextForRootTag.get(rootTag);
String component = getFabricComponentName(componentName);
synchronized (mPreMountItemsLock) {
@@ -342,10 +346,12 @@ public class FabricUIManager implements UIManager, LifecycleEventListener {
int reactTag,
boolean isLayoutable) {
String component = getFabricComponentName(componentName);
ThemedReactContext reactContext = mReactContextForRootTag.get(reactRootTag);
if (reactContext == null) {
throw new IllegalArgumentException("Unable to find ReactContext for root: " + reactRootTag);
}
// This could be null if teardown/navigation away from a surface on the main thread happens
// while a commit is being processed in a different thread. By contract we expect this to be
// possible at teardown, but this race should *never* happen at startup.
@Nullable ThemedReactContext reactContext = mReactContextForRootTag.get(reactRootTag);
return new CreateMountItem(
reactContext,
reactRootTag,
@@ -25,7 +25,7 @@ public class CreateMountItem implements MountItem {
private final boolean mIsLayoutable;
public CreateMountItem(
@NonNull ThemedReactContext context,
@Nullable ThemedReactContext context,
int rootTag,
int reactTag,
@NonNull String component,
@@ -43,6 +43,13 @@ public class CreateMountItem implements MountItem {
@Override
public void execute(@NonNull MountingManager mountingManager) {
if (mContext == null) {
throw new IllegalStateException(
"Cannot execute PreAllocateViewMountItem without Context for ReactTag: "
+ mReactTag
+ " and rootTag: "
+ mRootTag);
}
mountingManager.createView(
mContext, mComponent, mReactTag, mProps, mStateWrapper, mIsLayoutable);
}
@@ -31,7 +31,7 @@ public class PreAllocateViewMountItem implements MountItem {
private final boolean mIsLayoutable;
public PreAllocateViewMountItem(
@NonNull ThemedReactContext context,
@Nullable ThemedReactContext context,
int rootTag,
int reactTag,
@NonNull String component,
@@ -56,6 +56,13 @@ public class PreAllocateViewMountItem implements MountItem {
if (ENABLE_FABRIC_LOGS) {
FLog.d(TAG, "Executing pre-allocation of: " + toString());
}
if (mContext == null) {
throw new IllegalStateException(
"Cannot execute PreAllocateViewMountItem without Context for ReactTag: "
+ mReactTag
+ " and rootTag: "
+ mRootTag);
}
mountingManager.preallocateView(
mContext, mComponent, mReactTag, mProps, mStateWrapper, mIsLayoutable);
}