Skip to content

Commit

Permalink
Now using a thread lock on UIImplementation methods that mutate the n…
Browse files Browse the repository at this point in the history 
…ode children sparse array

- Fixes: facebook#17178 (comment)
  • Loading branch information
@SudoPlz
SudoPlz committed Feb 21, 2019
1 parent f32dc63 commit c0e1be5
Showing 1 changed file with 139 additions and 130 deletions.
269 changes: 139 additions & 130 deletions ReactAndroid/src/main/java/com/facebook/react/uimanager/UIImplementation.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
* shadow node hierarchy that is then mapped to a native view hierarchy.
*/
public class UIImplementation {
protected Object uiImplementationThreadLock = new Object();

protected final EventDispatcher mEventDispatcher;
protected final ReactApplicationContext mReactContext;
Expand Down Expand Up @@ -197,21 +198,22 @@ public void updateRootView(
*/
public <T extends View & MeasureSpecProvider> void registerRootView(
T rootView, int tag, ThemedReactContext context) {
final ReactShadowNode rootCSSNode = createRootShadowNode();
rootCSSNode.setReactTag(tag);
rootCSSNode.setThemedContext(context);

int widthMeasureSpec = rootView.getWidthMeasureSpec();
int heightMeasureSpec = rootView.getHeightMeasureSpec();
updateRootView(rootCSSNode, widthMeasureSpec, heightMeasureSpec);

context.runOnNativeModulesQueueThread(new Runnable() {
@Override
public void run() {
mShadowNodeRegistry.addRootNode(rootCSSNode);
}
});

synchronized (uiImplementationThreadLock) {
final ReactShadowNode rootCSSNode = createRootShadowNode();
rootCSSNode.setReactTag(tag);
rootCSSNode.setThemedContext(context);

int widthMeasureSpec = rootView.getWidthMeasureSpec();
int heightMeasureSpec = rootView.getHeightMeasureSpec();
updateRootView(rootCSSNode, widthMeasureSpec, heightMeasureSpec);

context.runOnNativeModulesQueueThread(new Runnable() {
@Override
public void run() {
mShadowNodeRegistry.addRootNode(rootCSSNode);
}
});
}
// register it within NativeViewHierarchyManager
mOperationsQueue.addRootView(tag, rootView);
}
Expand All @@ -228,7 +230,9 @@ public void removeRootView(int rootViewTag) {
* Unregisters a root node with a given tag from the shadow node registry
*/
public void removeRootShadowNode(int rootViewTag) {
mShadowNodeRegistry.removeRootNode(rootViewTag);
synchronized (uiImplementationThreadLock) {
mShadowNodeRegistry.removeRootNode(rootViewTag); // Thread safety needed here
}
}

/**
Expand Down Expand Up @@ -279,23 +283,25 @@ public Map<String, Long> getProfiledBatchPerfCounters() {
* Invoked by React to create a new node with a given tag, class name and properties.
*/
public void createView(int tag, String className, int rootViewTag, ReadableMap props) {
ReactShadowNode cssNode = createShadowNode(className);
ReactShadowNode rootNode = mShadowNodeRegistry.getNode(rootViewTag);
Assertions.assertNotNull(rootNode, "Root node with tag " + rootViewTag + " doesn't exist");
cssNode.setReactTag(tag);
cssNode.setViewClassName(className);
cssNode.setRootTag(rootNode.getReactTag());
cssNode.setThemedContext(rootNode.getThemedContext());

mShadowNodeRegistry.addNode(cssNode);
synchronized (uiImplementationThreadLock) {
ReactShadowNode cssNode = createShadowNode(className);
ReactShadowNode rootNode = mShadowNodeRegistry.getNode(rootViewTag);
Assertions.assertNotNull(rootNode, "Root node with tag " + rootViewTag + " doesn't exist");
cssNode.setReactTag(tag); // Thread safety needed here
cssNode.setViewClassName(className);
cssNode.setRootTag(rootNode.getReactTag());
cssNode.setThemedContext(rootNode.getThemedContext());

mShadowNodeRegistry.addNode(cssNode);

ReactStylesDiffMap styles = null;
if (props != null) {
styles = new ReactStylesDiffMap(props);
cssNode.updateProperties(styles);
}

ReactStylesDiffMap styles = null;
if (props != null) {
styles = new ReactStylesDiffMap(props);
cssNode.updateProperties(styles);
handleCreateView(cssNode, rootViewTag, styles);
}

handleCreateView(cssNode, rootViewTag, styles);
}

protected void handleCreateView(
Expand Down Expand Up @@ -363,106 +369,108 @@ public void manageChildren(
@Nullable ReadableArray addChildTags,
@Nullable ReadableArray addAtIndices,
@Nullable ReadableArray removeFrom) {
ReactShadowNode cssNodeToManage = mShadowNodeRegistry.getNode(viewTag);
synchronized (uiImplementationThreadLock) {
ReactShadowNode cssNodeToManage = mShadowNodeRegistry.getNode(viewTag);

int numToMove = moveFrom == null ? 0 : moveFrom.size();
int numToAdd = addChildTags == null ? 0 : addChildTags.size();
int numToRemove = removeFrom == null ? 0 : removeFrom.size();
int numToMove = moveFrom == null ? 0 : moveFrom.size();
int numToAdd = addChildTags == null ? 0 : addChildTags.size();
int numToRemove = removeFrom == null ? 0 : removeFrom.size();

if (numToMove != 0 && (moveTo == null || numToMove != moveTo.size())) {
throw new IllegalViewOperationException("Size of moveFrom != size of moveTo!");
}
if (numToMove != 0 && (moveTo == null || numToMove != moveTo.size())) {
throw new IllegalViewOperationException("Size of moveFrom != size of moveTo!");
}

if (numToAdd != 0 && (addAtIndices == null || numToAdd != addAtIndices.size())) {
throw new IllegalViewOperationException("Size of addChildTags != size of addAtIndices!");
}
if (numToAdd != 0 && (addAtIndices == null || numToAdd != addAtIndices.size())) {
throw new IllegalViewOperationException("Size of addChildTags != size of addAtIndices!");
}

// We treat moves as an add and a delete
ViewAtIndex[] viewsToAdd = new ViewAtIndex[numToMove + numToAdd];
int[] indicesToRemove = new int[numToMove + numToRemove];
int[] tagsToRemove = new int[indicesToRemove.length];
int[] tagsToDelete = new int[numToRemove];

if (numToMove > 0) {
Assertions.assertNotNull(moveFrom);
Assertions.assertNotNull(moveTo);
for (int i = 0; i < numToMove; i++) {
int moveFromIndex = moveFrom.getInt(i);
int tagToMove = cssNodeToManage.getChildAt(moveFromIndex).getReactTag();
viewsToAdd[i] = new ViewAtIndex(
tagToMove,
moveTo.getInt(i));
indicesToRemove[i] = moveFromIndex;
tagsToRemove[i] = tagToMove;
// We treat moves as an add and a delete
ViewAtIndex[] viewsToAdd = new ViewAtIndex[numToMove + numToAdd];
int[] indicesToRemove = new int[numToMove + numToRemove];
int[] tagsToRemove = new int[indicesToRemove.length];
int[] tagsToDelete = new int[numToRemove];

if (numToMove > 0) {
Assertions.assertNotNull(moveFrom);
Assertions.assertNotNull(moveTo);
for (int i = 0; i < numToMove; i++) {
int moveFromIndex = moveFrom.getInt(i);
int tagToMove = cssNodeToManage.getChildAt(moveFromIndex).getReactTag();
viewsToAdd[i] = new ViewAtIndex(
tagToMove,
moveTo.getInt(i));
indicesToRemove[i] = moveFromIndex;
tagsToRemove[i] = tagToMove;
}
}
}

if (numToAdd > 0) {
Assertions.assertNotNull(addChildTags);
Assertions.assertNotNull(addAtIndices);
for (int i = 0; i < numToAdd; i++) {
int viewTagToAdd = addChildTags.getInt(i);
int indexToAddAt = addAtIndices.getInt(i);
viewsToAdd[numToMove + i] = new ViewAtIndex(viewTagToAdd, indexToAddAt);
if (numToAdd > 0) {
Assertions.assertNotNull(addChildTags);
Assertions.assertNotNull(addAtIndices);
for (int i = 0; i < numToAdd; i++) {
int viewTagToAdd = addChildTags.getInt(i);
int indexToAddAt = addAtIndices.getInt(i);
viewsToAdd[numToMove + i] = new ViewAtIndex(viewTagToAdd, indexToAddAt);
}
}
}

if (numToRemove > 0) {
Assertions.assertNotNull(removeFrom);
for (int i = 0; i < numToRemove; i++) {
int indexToRemove = removeFrom.getInt(i);
int tagToRemove = cssNodeToManage.getChildAt(indexToRemove).getReactTag();
indicesToRemove[numToMove + i] = indexToRemove;
tagsToRemove[numToMove + i] = tagToRemove;
tagsToDelete[i] = tagToRemove;
if (numToRemove > 0) {
Assertions.assertNotNull(removeFrom);
for (int i = 0; i < numToRemove; i++) {
int indexToRemove = removeFrom.getInt(i);
int tagToRemove = cssNodeToManage.getChildAt(indexToRemove).getReactTag();
indicesToRemove[numToMove + i] = indexToRemove;
tagsToRemove[numToMove + i] = tagToRemove;
tagsToDelete[i] = tagToRemove;
}
}
}

// NB: moveFrom and removeFrom are both relative to the starting state of the View's children.
// moveTo and addAt are both relative to the final state of the View's children.
//
// 1) Sort the views to add and indices to remove by index
// 2) Iterate the indices being removed from high to low and remove them. Going high to low
// makes sure we remove the correct index when there are multiple to remove.
// 3) Iterate the views being added by index low to high and add them. Like the view removal,
// iteration direction is important to preserve the correct index.

Arrays.sort(viewsToAdd, ViewAtIndex.COMPARATOR);
Arrays.sort(indicesToRemove);

// Apply changes to CSSNodeDEPRECATED hierarchy
int lastIndexRemoved = -1;
for (int i = indicesToRemove.length - 1; i >= 0; i--) {
int indexToRemove = indicesToRemove[i];
if (indexToRemove == lastIndexRemoved) {
throw new IllegalViewOperationException("Repeated indices in Removal list for view tag: "
+ viewTag);
// NB: moveFrom and removeFrom are both relative to the starting state of the View's children.
// moveTo and addAt are both relative to the final state of the View's children.
//
// 1) Sort the views to add and indices to remove by index
// 2) Iterate the indices being removed from high to low and remove them. Going high to low
// makes sure we remove the correct index when there are multiple to remove.
// 3) Iterate the views being added by index low to high and add them. Like the view removal,
// iteration direction is important to preserve the correct index.

Arrays.sort(viewsToAdd, ViewAtIndex.COMPARATOR);
Arrays.sort(indicesToRemove);

// Apply changes to CSSNodeDEPRECATED hierarchy
int lastIndexRemoved = -1;
for (int i = indicesToRemove.length - 1; i >= 0; i--) {
int indexToRemove = indicesToRemove[i];
if (indexToRemove == lastIndexRemoved) {
throw new IllegalViewOperationException("Repeated indices in Removal list for view tag: "
+ viewTag);
}
cssNodeToManage.removeChildAt(indicesToRemove[i]); // Thread safety needed here
lastIndexRemoved = indicesToRemove[i];
}
cssNodeToManage.removeChildAt(indicesToRemove[i]);
lastIndexRemoved = indicesToRemove[i];
}

for (int i = 0; i < viewsToAdd.length; i++) {
ViewAtIndex viewAtIndex = viewsToAdd[i];
ReactShadowNode cssNodeToAdd = mShadowNodeRegistry.getNode(viewAtIndex.mTag);
if (cssNodeToAdd == null) {
throw new IllegalViewOperationException("Trying to add unknown view tag: "
+ viewAtIndex.mTag);
for (int i = 0; i < viewsToAdd.length; i++) {
ViewAtIndex viewAtIndex = viewsToAdd[i];
ReactShadowNode cssNodeToAdd = mShadowNodeRegistry.getNode(viewAtIndex.mTag);
if (cssNodeToAdd == null) {
throw new IllegalViewOperationException("Trying to add unknown view tag: "
+ viewAtIndex.mTag);
}
cssNodeToManage.addChildAt(cssNodeToAdd, viewAtIndex.mIndex);
}
cssNodeToManage.addChildAt(cssNodeToAdd, viewAtIndex.mIndex);
}

if (!cssNodeToManage.isVirtual() && !cssNodeToManage.isVirtualAnchor()) {
mNativeViewHierarchyOptimizer.handleManageChildren(
cssNodeToManage,
indicesToRemove,
tagsToRemove,
viewsToAdd,
tagsToDelete);
}
if (!cssNodeToManage.isVirtual() && !cssNodeToManage.isVirtualAnchor()) {
mNativeViewHierarchyOptimizer.handleManageChildren(
cssNodeToManage,
indicesToRemove,
tagsToRemove,
viewsToAdd,
tagsToDelete);
}

for (int i = 0; i < tagsToDelete.length; i++) {
removeShadowNode(mShadowNodeRegistry.getNode(tagsToDelete[i]));
for (int i = 0; i < tagsToDelete.length; i++) {
removeShadowNode(mShadowNodeRegistry.getNode(tagsToDelete[i]));
}
}
}

Expand All @@ -476,22 +484,23 @@ public void manageChildren(
public void setChildren(
int viewTag,
ReadableArray childrenTags) {

ReactShadowNode cssNodeToManage = mShadowNodeRegistry.getNode(viewTag);

for (int i = 0; i < childrenTags.size(); i++) {
ReactShadowNode cssNodeToAdd = mShadowNodeRegistry.getNode(childrenTags.getInt(i));
if (cssNodeToAdd == null) {
throw new IllegalViewOperationException("Trying to add unknown view tag: "
+ childrenTags.getInt(i));
synchronized (uiImplementationThreadLock) {
ReactShadowNode cssNodeToManage = mShadowNodeRegistry.getNode(viewTag);

for (int i = 0; i < childrenTags.size(); i++) {
ReactShadowNode cssNodeToAdd = mShadowNodeRegistry.getNode(childrenTags.getInt(i));
if (cssNodeToAdd == null) {
throw new IllegalViewOperationException("Trying to add unknown view tag: "
+ childrenTags.getInt(i));
}
cssNodeToManage.addChildAt(cssNodeToAdd, i);
}
cssNodeToManage.addChildAt(cssNodeToAdd, i);
}

if (!cssNodeToManage.isVirtual() && !cssNodeToManage.isVirtualAnchor()) {
mNativeViewHierarchyOptimizer.handleSetChildren(
cssNodeToManage,
childrenTags);
if (!cssNodeToManage.isVirtual() && !cssNodeToManage.isVirtualAnchor()) {
mNativeViewHierarchyOptimizer.handleSetChildren(
cssNodeToManage,
childrenTags);
}
}
}

Expand Down

0 comments on commit c0e1be5

Please sign in to comment.