David Vacca 79cc1468bc Fix race condition in Fabric Scheduler
Summary:
This diff fixes a race condition that was detected on "Marketplace You" production test for Android.
The race condition happens when the method ShadowTree::complete is executed concurrently from two threads (in this case this is called from Scheduller::constraintSurfaceLayout and Scheduler::uiManagerDidFinishTransaction), based on the order of execution this bug makes MountViewItems to be dispatched to the UI in the wrong order.

The root cause of the bug is in the method:

```
bool ShadowTree::complete(
    const SharedRootShadowNode &oldRootShadowNode,
    const UnsharedRootShadowNode &newRootShadowNode) const {
  newRootShadowNode->layout();
  newRootShadowNode->sealRecursive();

  auto mutations =
      calculateShadowViewMutations(*oldRootShadowNode, *newRootShadowNode);

  if (!commit(oldRootShadowNode, newRootShadowNode, mutations)) {
    return false;
  }

  emitLayoutEvents(mutations);

  if (delegate_) {
    delegate_->shadowTreeDidCommit(*this, mutations);
  }

  return true;
}
```
Notes:
- the commit method is guarded by the commitMutex_
- the shadowTreeDidCommit() method dispatches mutations instructions to the platform side.
- If there are two threads running concurrently, there is no guarantee that the shadowTreeDidCommit() is going to be called in the correct order.

The solution is to include the execution to shadowTreeDidCommit() in the same commitMutex_

Possible solutions:
1 - move the commitMutex_ out of the commit method (to the completeMethod)
2 - synchronize the call to complete method() - this is the implemented solution.
I chose this solution to make it consistent with the way Scheduler::constraintSurfaceLayout is implemented (https://fburl.com/8l49no5x)

This mechanism is very likely to change in the refactor of threading mechanism that Valentin Shergin is going to be working on January.

I would like to land this, so we can fix this bug and run another experiment in production as soon as possible.

Reviewed By: sahrens

Differential Revision: D13535587

fbshipit-source-id: bedd4d85f5569ab3733c302d1328aa48017bcaad
2018-12-26 11:15:34 -08:00
..
2018-11-27 18:03:27 -08:00