From b72d6768e0228792a30d1cd3874e99361a3c7f60 Mon Sep 17 00:00:00 2001 From: Valentin Shergin Date: Sun, 17 May 2020 14:59:16 -0700 Subject: [PATCH] Fabric: Automatic removing outstanding Surface on Scheduler destruction; 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 --- ReactCommon/fabric/scheduler/Scheduler.cpp | 10 ++++++++++ ReactCommon/fabric/scheduler/Scheduler.h | 4 ++++ 2 files changed, 14 insertions(+) diff --git a/ReactCommon/fabric/scheduler/Scheduler.cpp b/ReactCommon/fabric/scheduler/Scheduler.cpp index cc67b567091679..1acdf65a174ae7 100644 --- a/ReactCommon/fabric/scheduler/Scheduler.cpp +++ b/ReactCommon/fabric/scheduler/Scheduler.cpp @@ -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 } @@ -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); + } } } diff --git a/ReactCommon/fabric/scheduler/Scheduler.h b/ReactCommon/fabric/scheduler/Scheduler.h index c9f664f401e6c8..243db58a0e7c4d 100644 --- a/ReactCommon/fabric/scheduler/Scheduler.h +++ b/ReactCommon/fabric/scheduler/Scheduler.h @@ -121,7 +121,11 @@ class Scheduler final : public UIManagerDelegate { */ std::shared_ptr> eventDispatcher_; + /* + * Temporary flags. + */ bool enableNewStateReconciliation_{false}; + bool removeOutstandingSurfacesOnDestruction_{false}; }; } // namespace react