Skip to content

Commit

Permalink
Fabric: Automatic removing outstanding Surface on Scheduler destructi…
Browse files Browse the repository at this point in the history
…on; gated.

Summary:
This is an addition to an automatic emergency clean-up algorithm that we have in Scheduler. In addition to committing empty surfaces, we also remove those surfaces from the registry making calling stuff on them impossible. Removing surfaces waits for all commits in flight to be finished, so it theoretically can deadlock (so we gated that).

If we won't face deadlocks in a coming couple of weeks, I would remove gating.

Changelog: [Internal] Fabric-specific internal change.

Reviewed By: mdvacca

Differential Revision: D21610683

fbshipit-source-id: 71feeaa0ee4521a0180cdfba6e3a271e7f7d9401
  • Loading branch information
shergin authored and facebook-github-bot committed May 17, 2020
1 parent 6a96a9f commit b72d676
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 0 deletions.
10 changes: 10 additions & 0 deletions ReactCommon/fabric/scheduler/Scheduler.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,13 @@ Scheduler::Scheduler(
#ifdef ANDROID
enableNewStateReconciliation_ = reactNativeConfig_->getBool(
"react_fabric:enable_new_state_reconciliation_android");
removeOutstandingSurfacesOnDestruction_ = reactNativeConfig_->getBool(
"react_fabric:remove_outstanding_surfaces_on_destruction_android");
#else
enableNewStateReconciliation_ = reactNativeConfig_->getBool(
"react_fabric:enable_new_state_reconciliation_ios");
removeOutstandingSurfacesOnDestruction_ = reactNativeConfig_->getBool(
"react_fabric:remove_outstanding_surfaces_on_destruction_ios");
#endif
}

Expand Down Expand Up @@ -140,6 +144,12 @@ Scheduler::~Scheduler() {
uiManager_->getShadowTreeRegistry().visit(
surfaceId,
[](ShadowTree const &shadowTree) { shadowTree.commitEmptyTree(); });

// Removing surfaces is gated because it acquires mutex waiting for commits
// in flight; in theory, it can deadlock.
if (removeOutstandingSurfacesOnDestruction_) {
uiManager_->getShadowTreeRegistry().remove(surfaceId);
}
}
}

Expand Down
4 changes: 4 additions & 0 deletions ReactCommon/fabric/scheduler/Scheduler.h
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,11 @@ class Scheduler final : public UIManagerDelegate {
*/
std::shared_ptr<better::optional<EventDispatcher const>> eventDispatcher_;

/*
* Temporary flags.
*/
bool enableNewStateReconciliation_{false};
bool removeOutstandingSurfacesOnDestruction_{false};
};

} // namespace react
Expand Down

0 comments on commit b72d676

Please sign in to comment.