From 5f4aa6ae4274dedacc86b306bb0cf23e24e2c6c9 Mon Sep 17 00:00:00 2001
From: Valentin Shergin <shergin@fb.com>
Date: Mon, 8 Oct 2018 14:34:25 -0700
Subject: [PATCH] Fabric: Proper failCallback handling in
 EventBeatBasedExecutor

Summary: That's why we need the previous three diffs. Synchonous executor deadlocks if the beat is missing.

Reviewed By: sahrens

Differential Revision: D10081501

fbshipit-source-id: 9986d0a1844e642048b6f37a1fcb5f623a267663
---
 React/Fabric/Utils/MessageQueueEventBeat.mm          | 3 +++
 ReactCommon/fabric/events/EventBeatBasedExecutor.cpp | 9 ++++++---
 ReactCommon/fabric/events/EventBeatBasedExecutor.h   | 2 +-
 3 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/React/Fabric/Utils/MessageQueueEventBeat.mm b/React/Fabric/Utils/MessageQueueEventBeat.mm
index 49099f443..c6b87dd34 100644
--- a/React/Fabric/Utils/MessageQueueEventBeat.mm
+++ b/React/Fabric/Utils/MessageQueueEventBeat.mm
@@ -46,6 +46,9 @@ void MessageQueueEventBeat::induce() const {
   // If `wasExecuted` was destroyed before set to `true`,
   // it means that the execution block was deallocated not being executed.
   // This indicates that `messageQueueThread_` is being deallocated.
+  // This trick is quite expensive due to deallocation and messing with atomic
+  // counters. Seems we need this only for making hot-reloading mechanism
+  // thread-safe. Hence, let's leave it to be DEBUG-only for now.
   auto wasExecuted = std::shared_ptr<bool>(new bool {false}, [this](bool *wasExecuted) {
     if (!*wasExecuted && failCallback_) {
       failCallback_();
diff --git a/ReactCommon/fabric/events/EventBeatBasedExecutor.cpp b/ReactCommon/fabric/events/EventBeatBasedExecutor.cpp
index 13c15080a..eaef666cf 100644
--- a/ReactCommon/fabric/events/EventBeatBasedExecutor.cpp
+++ b/ReactCommon/fabric/events/EventBeatBasedExecutor.cpp
@@ -19,7 +19,8 @@ using Mode = EventBeatBasedExecutor::Mode;
 EventBeatBasedExecutor::EventBeatBasedExecutor(std::unique_ptr<EventBeat> eventBeat):
   eventBeat_(std::move(eventBeat)) {
 
-  eventBeat_->setBeatCallback(std::bind(&EventBeatBasedExecutor::onBeat, this));
+  eventBeat_->setBeatCallback(std::bind(&EventBeatBasedExecutor::onBeat, this, true));
+  eventBeat_->setFailCallback(std::bind(&EventBeatBasedExecutor::onBeat, this, false));
 }
 
 void EventBeatBasedExecutor::operator()(Routine routine, Mode mode) const {
@@ -52,7 +53,7 @@ void EventBeatBasedExecutor::execute(Task task) const {
   eventBeat_->induce();
 }
 
-void EventBeatBasedExecutor::onBeat() const {
+void EventBeatBasedExecutor::onBeat(bool success) const {
   std::vector<Task> tasks;
 
   {
@@ -67,7 +68,9 @@ void EventBeatBasedExecutor::onBeat() const {
   }
 
   for (const auto task : tasks) {
-    task.routine();
+    if (success) {
+      task.routine();
+    }
 
     if (task.callback) {
       task.callback();
diff --git a/ReactCommon/fabric/events/EventBeatBasedExecutor.h b/ReactCommon/fabric/events/EventBeatBasedExecutor.h
index cc8b9b254..52e69026e 100644
--- a/ReactCommon/fabric/events/EventBeatBasedExecutor.h
+++ b/ReactCommon/fabric/events/EventBeatBasedExecutor.h
@@ -43,7 +43,7 @@ public:
   void operator()(Routine routine, Mode mode = Mode::Asynchronous) const;
 
 private:
-  void onBeat() const;
+  void onBeat(bool success = true) const;
   void execute(Task task) const;
 
   std::unique_ptr<EventBeat> eventBeat_;