From 89faf0c9a87f6de68ca416d10566dbcbe80d9450 Mon Sep 17 00:00:00 2001
From: Marshall Mann-Wood <mrmannwood@fb.com>
Date: Wed, 15 Dec 2021 13:33:56 -0800
Subject: [PATCH] Added saftey for ensuring callback is called

Summary:
Adds a return value to `MessageQueueThread#runOnQueue`, it's implementors and one caller.

It is currently possible for a callback to not be called (because the HandlerThread has been shutdown). If the callback is mission-critical (frees resources, releases a lock, etc) the callee has no indication that it needs to do something.

This surfaces that information to the callee.

Changelog:
[Android][Changed] - Made `MessageQueueThread#runOnQueue` return a boolean. Made `MessageQueueThreadImpl#runOnQueue` return false when the runnable is not submitted.

Reviewed By: RSNara

Differential Revision: D33075516

fbshipit-source-id: bd214a39ae066c0e7d413f2eaaaa05bd072ead2a
---
 .../src/main/java/com/facebook/react/bridge/ReactContext.java | 4 ++--
 .../com/facebook/react/bridge/queue/MessageQueueThread.java   | 2 +-
 .../facebook/react/bridge/queue/MessageQueueThreadImpl.java   | 4 +++-
 ReactAndroid/src/main/jni/react/jni/JMessageQueueThread.cpp   | 2 +-
 4 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContext.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContext.java
index dbc51207af2209..24b4396e8ae88d 100644
--- a/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContext.java
+++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/ReactContext.java
@@ -396,8 +396,8 @@ public boolean isOnJSQueueThread() {
     return Assertions.assertNotNull(mJSMessageQueueThread).isOnThread();
   }
 
-  public void runOnJSQueueThread(Runnable runnable) {
-    Assertions.assertNotNull(mJSMessageQueueThread).runOnQueue(runnable);
+  public boolean runOnJSQueueThread(Runnable runnable) {
+    return Assertions.assertNotNull(mJSMessageQueueThread).runOnQueue(runnable);
   }
 
   /**
diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/queue/MessageQueueThread.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/queue/MessageQueueThread.java
index f40885e8ac913c..563df0304c7e2b 100644
--- a/ReactAndroid/src/main/java/com/facebook/react/bridge/queue/MessageQueueThread.java
+++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/queue/MessageQueueThread.java
@@ -19,7 +19,7 @@ public interface MessageQueueThread {
    * if it is being submitted from the same queue Thread.
    */
   @DoNotStrip
-  void runOnQueue(Runnable runnable);
+  boolean runOnQueue(Runnable runnable);
 
   /**
    * Runs the given Callable on this Thread. It will be submitted to the end of the event queue even
diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/queue/MessageQueueThreadImpl.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/queue/MessageQueueThreadImpl.java
index 00beffd3a558aa..5ad08392779a94 100644
--- a/ReactAndroid/src/main/java/com/facebook/react/bridge/queue/MessageQueueThreadImpl.java
+++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/queue/MessageQueueThreadImpl.java
@@ -55,15 +55,17 @@ private MessageQueueThreadImpl(
    */
   @DoNotStrip
   @Override
-  public void runOnQueue(Runnable runnable) {
+  public boolean runOnQueue(Runnable runnable) {
     if (mIsFinished) {
       FLog.w(
           ReactConstants.TAG,
           "Tried to enqueue runnable on already finished thread: '"
               + getName()
               + "... dropping Runnable.");
+      return false;
     }
     mHandler.post(runnable);
+    return true;
   }
 
   @DoNotStrip
diff --git a/ReactAndroid/src/main/jni/react/jni/JMessageQueueThread.cpp b/ReactAndroid/src/main/jni/react/jni/JMessageQueueThread.cpp
index badb136f4c22f5..9b889aa3156a6c 100644
--- a/ReactAndroid/src/main/jni/react/jni/JMessageQueueThread.cpp
+++ b/ReactAndroid/src/main/jni/react/jni/JMessageQueueThread.cpp
@@ -69,7 +69,7 @@ void JMessageQueueThread::runOnQueue(std::function<void()> &&runnable) {
   jni::ThreadScope guard;
   static auto method =
       JavaMessageQueueThread::javaClassStatic()
-          ->getMethod<void(Runnable::javaobject)>("runOnQueue");
+          ->getMethod<jboolean(Runnable::javaobject)>("runOnQueue");
   method(
       m_jobj,
       JNativeRunnable::newObjectCxxArgs(wrapRunnable(std::move(runnable)))