From 81d0f9a6909367859601cd0b60e61033eab3d0dc Mon Sep 17 00:00:00 2001
From: David Vacca <dvacca@fb.com>
Date: Mon, 29 Apr 2019 18:08:39 -0700
Subject: [PATCH] Ensure proper Synchronization on ReactChoreographer

Summary: This diff refactors the way we synchronize in ReactChoreographer using a lock object

Reviewed By: ejanzer

Differential Revision: D14913056

fbshipit-source-id: e86c4395d5d3c3fd5b7330b72c14920b536f74ce
---
 .../modules/core/ReactChoreographer.java      | 38 ++++++++++---------
 1 file changed, 21 insertions(+), 17 deletions(-)

diff --git a/ReactAndroid/src/main/java/com/facebook/react/modules/core/ReactChoreographer.java b/ReactAndroid/src/main/java/com/facebook/react/modules/core/ReactChoreographer.java
index 283556cea..52fe20f44 100644
--- a/ReactAndroid/src/main/java/com/facebook/react/modules/core/ReactChoreographer.java
+++ b/ReactAndroid/src/main/java/com/facebook/react/modules/core/ReactChoreographer.java
@@ -79,6 +79,7 @@ public class ReactChoreographer {
   private @Nullable volatile ChoreographerCompat mChoreographer;
   private final ReactChoreographerDispatcher mReactChoreographerDispatcher;
   private final ArrayDeque<ChoreographerCompat.FrameCallback>[] mCallbackQueues;
+  private final Object mCallbackQueuesLock = new Object();
 
   private int mTotalCallbacks = 0;
   private boolean mHasPostedCallback = false;
@@ -92,22 +93,25 @@ public class ReactChoreographer {
     initializeChoreographer(null);
   }
 
-  public synchronized void postFrameCallback(
+  public void postFrameCallback(
     CallbackType type,
     ChoreographerCompat.FrameCallback frameCallback) {
-    mCallbackQueues[type.getOrder()].addLast(frameCallback);
-    mTotalCallbacks++;
-    Assertions.assertCondition(mTotalCallbacks > 0);
-    if (!mHasPostedCallback) {
-      if (mChoreographer == null) {
-        initializeChoreographer(new Runnable(){
-          @Override
-          public void run() {
-            postFrameCallbackOnChoreographer();
-          }
-        });
-      } else {
-        postFrameCallbackOnChoreographer();
+    synchronized (mCallbackQueuesLock) {
+      mCallbackQueues[type.getOrder()].addLast(frameCallback);
+      mTotalCallbacks++;
+      Assertions.assertCondition(mTotalCallbacks > 0);
+      if (!mHasPostedCallback) {
+        if (mChoreographer == null) {
+          initializeChoreographer(
+              new Runnable() {
+                @Override
+                public void run() {
+                  postFrameCallbackOnChoreographer();
+                }
+              });
+        } else {
+          postFrameCallbackOnChoreographer();
+        }
       }
     }
   }
@@ -133,10 +137,10 @@ public class ReactChoreographer {
     });
   }
 
-  public synchronized void removeFrameCallback(
+  public void removeFrameCallback(
     CallbackType type,
     ChoreographerCompat.FrameCallback frameCallback) {
-    synchronized (ReactChoreographer.this) {
+    synchronized (mCallbackQueuesLock) {
       if (mCallbackQueues[type.getOrder()].removeFirstOccurrence(frameCallback)) {
         mTotalCallbacks--;
         maybeRemoveFrameCallback();
@@ -160,7 +164,7 @@ public class ReactChoreographer {
 
     @Override
     public void doFrame(long frameTimeNanos) {
-      synchronized (ReactChoreographer.this) {
+      synchronized (mCallbackQueuesLock) {
         mHasPostedCallback = false;
         for (int i = 0; i < mCallbackQueues.length; i++) {
           int initialLength = mCallbackQueues[i].size();