mirror of
https://github.com/facebook/react-native.git
synced 2025-11-01 09:14:26 +00:00
Add debugging code to recover from, and debug errors where Views in the hierarchy are re-added to hierarchy
Summary: There is a very small volume of production errors caused by a View that is already in the hierarchy being added to the hierarchy again; this results in a crash on the Android platform layer. We detect and attempt to silently recover from this case, while logging and collecting more diagnostics. This will still crash in debug mode. It is unclear what layer this error is coming from: it could be an issue with the C++ differ (ordering, or something more tricky); that is unlikely but there are few other hypotheses at the moment. Changelog: [Internal] Reviewed By: javache Differential Revision: D37557663 fbshipit-source-id: ffe320ff646e314fa921a2c2cf8058254713505c
This commit is contained in:
committed by
Facebook GitHub Bot
parent
44aac0f797
commit
286b38eb53
+54
-1
@@ -84,6 +84,9 @@ public class SurfaceMountingManager {
|
||||
@ThreadConfined(UI)
|
||||
private final Stack<Integer> mReactTagsToRemove = new Stack<>();
|
||||
|
||||
@ThreadConfined(UI)
|
||||
private final Set<Integer> mErroneouslyReaddedReactTags = new HashSet<>();
|
||||
|
||||
@ThreadConfined(UI)
|
||||
private RemoveDeleteTreeUIFrameCallback mRemoveDeleteTreeUIFrameCallback;
|
||||
|
||||
@@ -378,7 +381,29 @@ public class SurfaceMountingManager {
|
||||
+ "]: View already has a parent: ["
|
||||
+ actualParentId
|
||||
+ "] "
|
||||
+ viewParent.getClass().getSimpleName()));
|
||||
+ " Parent: "
|
||||
+ viewParent.getClass().getSimpleName()
|
||||
+ " View: "
|
||||
+ view.getClass().getSimpleName()));
|
||||
|
||||
// We've hit an error case, and `addView` will crash below
|
||||
// if we don't take evasive action (it is an error to add a View
|
||||
// to the hierarchy if it already has a parent).
|
||||
// We don't know /why/ this happens yet, but it does happen
|
||||
// very infrequently in production.
|
||||
// Thus, we do three things here:
|
||||
// (1) We logged a SoftException above, so if there's a crash later
|
||||
// on, we might have some hints about what caused it.
|
||||
// (2) We remove the View from its parent.
|
||||
// (3) In case the View was removed from the hierarchy with the
|
||||
// RemoveDeleteTree instruction, and is now being readded - which
|
||||
// should be impossible - we mark this as a "readded" View and
|
||||
// thus prevent the RemoveDeleteTree worker from deleting this
|
||||
// View in the future.
|
||||
if (viewParent instanceof ViewGroup) {
|
||||
((ViewGroup) viewParent).removeView(view);
|
||||
}
|
||||
mErroneouslyReaddedReactTags.add(tag);
|
||||
}
|
||||
|
||||
try {
|
||||
@@ -421,6 +446,17 @@ public class SurfaceMountingManager {
|
||||
return;
|
||||
}
|
||||
|
||||
// This is "impossible". See comments above.
|
||||
if (mErroneouslyReaddedReactTags.contains(tag)) {
|
||||
ReactSoftExceptionLogger.logSoftException(
|
||||
TAG,
|
||||
new IllegalViewOperationException(
|
||||
"removeViewAt tried to remove a React View that was actually reused. This indicates a bug in the Differ (specifically instruction ordering). ["
|
||||
+ tag
|
||||
+ "]"));
|
||||
return;
|
||||
}
|
||||
|
||||
UiThreadUtil.assertOnUiThread();
|
||||
ViewState parentViewState = getNullableViewState(parentTag);
|
||||
|
||||
@@ -1426,6 +1462,17 @@ public class SurfaceMountingManager {
|
||||
int reactTag = mReactTagsToRemove.pop();
|
||||
deletedViews++;
|
||||
|
||||
// This is "impossible". See comments above.
|
||||
if (mErroneouslyReaddedReactTags.contains(reactTag)) {
|
||||
ReactSoftExceptionLogger.logSoftException(
|
||||
TAG,
|
||||
new IllegalViewOperationException(
|
||||
"RemoveDeleteTree recursively tried to remove a React View that was actually reused. This indicates a bug in the Differ. ["
|
||||
+ reactTag
|
||||
+ "]"));
|
||||
continue;
|
||||
}
|
||||
|
||||
ViewState thisViewState = getNullableViewState(reactTag);
|
||||
if (thisViewState != null) {
|
||||
View thisView = thisViewState.mView;
|
||||
@@ -1489,6 +1536,12 @@ public class SurfaceMountingManager {
|
||||
if (!mReactTagsToRemove.empty()) {
|
||||
ReactChoreographer.getInstance()
|
||||
.postFrameCallback(ReactChoreographer.CallbackType.IDLE_EVENT, this);
|
||||
} else {
|
||||
// If there are no more tags to process, then clear the "reused"
|
||||
// tag set. Since the RemoveDeleteTree runner executes /after/ all
|
||||
// other mounting instructions have been executed, all in-band Remove
|
||||
// instructions have already had a chance to execute here.
|
||||
mErroneouslyReaddedReactTags.clear();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user