From 52a9fed3dfad55f2e122ea77134cc58d3a4e588d Mon Sep 17 00:00:00 2001
From: Samuel Susla <samuelsusla@fb.com>
Date: Fri, 18 Jun 2021 14:51:29 -0700
Subject: [PATCH] Use atomic pointer for animationDelegate_ to prevent race
 during teardown

Summary:
Changelog: [internal]

[This assumption](https://fburl.com/diffusion/yeqtvxru) in `~Scheduler` might not be completely correct. There can be a race between JS thread and main thread. With main thread settings animationDelegate_ to nullptr and JS thread reading the value. Even though they always happen one after the other, JS thread can have the value of animationDelegate_ in its cache line and changing it from main thread might not invalidate it right away.

Reviewed By: JoshuaGross

Differential Revision: D29237290

fbshipit-source-id: 6cb898caf7c225cf8162e7560921b563dec514b1
---
 ReactCommon/react/renderer/uimanager/UIManager.cpp | 14 ++++++++------
 ReactCommon/react/renderer/uimanager/UIManager.h   |  2 +-
 2 files changed, 9 insertions(+), 7 deletions(-)

diff --git a/ReactCommon/react/renderer/uimanager/UIManager.cpp b/ReactCommon/react/renderer/uimanager/UIManager.cpp
index bc35cd74e82..b99cd3be700 100644
--- a/ReactCommon/react/renderer/uimanager/UIManager.cpp
+++ b/ReactCommon/react/renderer/uimanager/UIManager.cpp
@@ -345,8 +345,9 @@ void UIManager::configureNextLayoutAnimation(
     RawValue const &config,
     jsi::Value const &successCallback,
     jsi::Value const &failureCallback) const {
-  if (animationDelegate_) {
-    animationDelegate_->uiManagerDidConfigureNextLayoutAnimation(
+  auto animationDelegate = animationDelegate_.load();
+  if (animationDelegate) {
+    animationDelegate->uiManagerDidConfigureNextLayoutAnimation(
         runtime,
         config,
         std::move(successCallback),
@@ -435,14 +436,15 @@ void UIManager::setAnimationDelegate(UIManagerAnimationDelegate *delegate) {
 }
 
 void UIManager::stopSurfaceForAnimationDelegate(SurfaceId surfaceId) const {
-  if (animationDelegate_ != nullptr) {
-    animationDelegate_->stopSurface(surfaceId);
+  auto animationDelegate = animationDelegate_.load();
+  if (animationDelegate) {
+    animationDelegate->stopSurface(surfaceId);
   }
 }
 
 void UIManager::animationTick() {
-  if (animationDelegate_ != nullptr &&
-      animationDelegate_->shouldAnimateFrame()) {
+  auto animationDelegate = animationDelegate_.load();
+  if (animationDelegate && animationDelegate->shouldAnimateFrame()) {
     shadowTreeRegistry_.enumerate(
         [&](ShadowTree const &shadowTree, bool &stop) {
           shadowTree.notifyDelegatesOfUpdates();
diff --git a/ReactCommon/react/renderer/uimanager/UIManager.h b/ReactCommon/react/renderer/uimanager/UIManager.h
index 617410f4afd..285df897d1e 100644
--- a/ReactCommon/react/renderer/uimanager/UIManager.h
+++ b/ReactCommon/react/renderer/uimanager/UIManager.h
@@ -184,8 +184,8 @@ class UIManager final : public ShadowTreeDelegate {
   ShadowTreeRegistry const &getShadowTreeRegistry() const;
 
   SharedComponentDescriptorRegistry componentDescriptorRegistry_;
-  UIManagerAnimationDelegate *animationDelegate_{nullptr};
   std::atomic<UIManagerDelegate *> delegate_;
+  std::atomic<UIManagerAnimationDelegate *> animationDelegate_{nullptr};
   RuntimeExecutor const runtimeExecutor_{};
   ShadowTreeRegistry shadowTreeRegistry_{};
   BackgroundExecutor const backgroundExecutor_{};