Permalink
Browse files

Fix crash if native code tries to update the size of a modal view aft…

…er JS has removed it

Summary:
See #10845

onSizeChanged is enqueueing a runnable from the ui thread that references a shadow view by id on the native modules thread. Since the shadow thread 'runs ahead' of the UI thread (e.g. it has a newer state of the world than the UI thread), this isn't safe. By the time that code block runs, it's possible that an update from JS has already removed that view from the shadow hierarchy (a change which would then propagate to the native hierarchy on the UI thread).

Reviewed By: AaaChiuuu

Differential Revision: D4706521

fbshipit-source-id: 0915f081068709b895f70b2edce12163b39e5456
  • Loading branch information...
astreet authored and facebook-github-bot committed Mar 15, 2017
1 parent 6a9c244 commit 5873a228f41bca763c35c470375a9f3cb36f80ab
@@ -153,6 +153,12 @@ public void updateNodeSize(
int newWidth,
int newHeight) {
ReactShadowNode cssNode = mShadowNodeRegistry.getNode(nodeViewTag);
if (cssNode == null) {
FLog.w(
ReactConstants.TAG,
"Tried to update size of non-existent tag: " + nodeViewTag);
return;
}
cssNode.setStyleWidth(newWidth);
cssNode.setStyleHeight(newHeight);
@@ -304,13 +304,14 @@ public DialogRootViewGroup(Context context) {
protected void onSizeChanged(final int w, final int h, int oldw, int oldh) {
super.onSizeChanged(w, h, oldw, oldh);
if (getChildCount() > 0) {
final int viewTag = getChildAt(0).getId();
ReactContext reactContext = (ReactContext) getContext();
reactContext.runOnNativeModulesQueueThread(
new GuardedRunnable(reactContext) {
@Override
public void runGuarded() {
((ReactContext) getContext()).getNativeModule(UIManagerModule.class)
.updateNodeSize(getChildAt(0).getId(), w, h);
.updateNodeSize(viewTag, w, h);
}
});
}

2 comments on commit 5873a22

@gsaandy

This comment has been minimized.

Show comment
Hide comment
@gsaandy

gsaandy Apr 6, 2017

why this is not in v43.2?

gsaandy replied Apr 6, 2017

why this is not in v43.2?

@grabbou

This comment has been minimized.

Show comment
Hide comment
@grabbou

grabbou Apr 10, 2017

Collaborator

It's part of 0.43.3 (shipping now) as well as 0.44-rc.

Collaborator

grabbou replied Apr 10, 2017

It's part of 0.43.3 (shipping now) as well as 0.44-rc.

Please sign in to comment.