Skip to content

Commit

Permalink
Deferred tree deletion needs to take non-managed trees into account
Browse files Browse the repository at this point in the history
Summary:
Crashes can occur if we try to disassemble trees not managed by React Native - for example a native component tree, Litho hierarchies, etc.

As we disassemble the tree, ensure that children are managed before disassembling a subtree.

Changelog: [Internal]

Reviewed By: ryancat

Differential Revision: D37493854

fbshipit-source-id: fee4d303133edcef663abfe8637bce6b24627724
  • Loading branch information
JoshuaGross authored and facebook-github-bot committed Jun 28, 2022
1 parent 3a7170c commit 297b571
Showing 1 changed file with 33 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1430,23 +1430,48 @@ public void doFrameGuarded(long frameTimeNanos) {
if (thisViewState != null) {
View thisView = thisViewState.mView;
int numChildren = 0;

// Children are managed by React Native if both of the following are true:
// 1) There are 1 or more children of this View, which must be a ViewGroup
// 2) Those children are managed by RN (this is not the case for certain native
// components, like embedded Litho hierarchies)
boolean childrenAreManaged = false;

if (thisView instanceof ViewGroup) {
View nextChild = null;
// For reasons documented elsewhere in this class, getChildCount is not
// necessarily
// reliable, and so we rely instead on requesting children directly.
// necessarily reliable, and so we rely instead on requesting children directly.
while ((nextChild = ((ViewGroup) thisView).getChildAt(numChildren)) != null) {
if (numChildren == 0) {
// Push tag onto the stack so we reprocess it after all children
mReactTagsToRemove.push(reactTag);
}
int childId = nextChild.getId();
childrenAreManaged = childrenAreManaged || getNullableViewState(childId) != null;
mReactTagsToRemove.push(nextChild.getId());
numChildren++;
}
// Removing all at once is more efficient than removing one-by-one
((ViewGroup) thisView).removeAllViews();
// If the children are not managed by RN, we simply drop the entire
// subtree instead of recursing further.
if (childrenAreManaged) {
try {
// This can happen if the removeAllViews method is overriden to throw,
// which it is explicitly in some cases (for example embedded Litho views,
// but there could be other cases). In those cases, we want to fail silently
// and then assume the subtree is /not/ managed by React Native.
// In this case short-lived memory-leaks could occur if we aren't clearing
// out the ViewState map properly; but the risk should be small.
// In debug mode, the SoftException will cause a crash. In production it
// will not. This should give good visibility into whether or not this is
// a problem without causing user-facing errors.
((ViewGroup) thisView).removeAllViews();
} catch (RuntimeException e) {
childrenAreManaged = false;
ReactSoftExceptionLogger.logSoftException(TAG, e);
}
}
}
if (numChildren == 0) {
if (childrenAreManaged) {
// Push tag onto the stack so we reprocess it after all children
mReactTagsToRemove.push(reactTag);
} else {
mTagToViewState.remove(reactTag);
onViewStateDeleted(thisViewState);
}
Expand Down

0 comments on commit 297b571

Please sign in to comment.