Fixup deadlock when hammering futures of a certain type

Author: jhugman

cpp/includes/UniffiCallInvoker.h

@@ -5,9 +5,10 @@
  */
 #pragma once
 #include <ReactCommon/CallInvoker.h>
-#include <future>
+#include <condition_variable>
 #include <jsi/jsi.h>
 #include <memory>
+#include <mutex>
 #include <thread>
 
 namespace uniffi_runtime {
@@ -57,23 +58,26 @@ class UniffiCallInvoker {
     if (std::this_thread::get_id() == threadId_) {
       func(rt);
     } else {
-      std::promise<void> promise;
-      auto future = promise.get_future();
+      std::mutex mtx;
+      std::condition_variable cv;
+      bool done = false;
       // The runtime argument was added to CallFunc in
       // https://github.com/facebook/react-native/pull/43375
       //
-      // Once that is released, there will be a deprecation period.
-      //
-      // Any time during the deprecation period, we can switch `&rt`
-      // from being a captured variable to being an argument, i.e.
-      // commenting out one line, and uncommenting the other.
-      std::function<void()> wrapper = [&func, &promise, &rt]() {
-        // react::CallFunc wrapper = [&func, &promise](jsi::Runtime &rt) {
+      // This can be changed once that change is released.
+      // react::CallFunc wrapper = [&func, &mtx, &cv, &done](jsi::Runtime &rt) {
+      std::function<void()> wrapper = [&func, &rt, &mtx, &cv, &done]() {
         func(rt);
-        promise.set_value();
+        {
+          std::lock_guard<std::mutex> lock(mtx);
+          done = true;
+        }
+        cv.notify_one();
       };
       callInvoker_->invokeAsync(std::move(wrapper));
-      future.wait();
+
+      std::unique_lock<std::mutex> lock(mtx);
+      cv.wait(lock, [&done] { return done; });
     }
   }
 };

typescript/src/async-rust-call.ts

@@ -33,6 +33,12 @@ type PollFunc = (
   handle: UniffiHandle,
 ) => void;
 
+// Calls setTimeout and then resolves the promise.
+// This may be used as a simple yield.
+export async function delayPromise(delayMs: number): Promise<void> {
+  return new Promise((resolve) => setTimeout(resolve, delayMs));
+}
+
 /**
  * This method calls an asynchronous method on the Rust side.
  *
@@ -71,7 +77,7 @@ export async function uniffiRustCallAsync<F, T>(
       pollResult = await pollRust((handle) => {
         pollFunc(rustFuture, uniffiFutureContinuationCallback, handle);
       });
-    } while (pollResult != UNIFFI_RUST_FUTURE_POLL_READY);
+    } while (pollResult !== UNIFFI_RUST_FUTURE_POLL_READY);
 
     // Now it's ready, all we need to do is pick up the result (and error).
     return liftFunc(
@@ -85,7 +91,7 @@ export async function uniffiRustCallAsync<F, T>(
     // #RUST_TASK_CANCELLATION: the unused `cancelFunc` function should be exposed
     // to client code in order for clients to be able to cancel the running Rust task.
   } finally {
-    freeFunc(rustFuture);
+    setTimeout(() => freeFunc(rustFuture), 0);
   }
 }
 
@@ -110,7 +116,24 @@ const uniffiFutureContinuationCallback: UniffiRustFutureContinuationCallback = (
   pollResult: number,
 ) => {
   const resolve = UNIFFI_RUST_FUTURE_RESOLVER_MAP.remove(handle);
-  resolve(pollResult);
+  if (pollResult === UNIFFI_RUST_FUTURE_POLL_READY) {
+    resolve(pollResult);
+  } else {
+    // From https://github.com/mozilla/uniffi-rs/pull/1837/files#diff-8a28c9cf1245b4f714d406ea4044d68e1000099928eaca1afb504ccbc008fe9fR35-R37
+    //
+    // > WARNING: the call to [rust_future_poll] must be scheduled to happen soon after the callback is
+    // > called, but not inside the callback itself.  If [rust_future_poll] is called inside the
+    // > callback, some futures will deadlock and our scheduler code might as well.
+    //
+    // This delay is to ensure that `uniffiFutureContinuationCallback` returns before the next poll, i.e.
+    // so that the next poll is outside of this callback.
+    //
+    // The length of the delay seems to be significant (at least in tests which hammer a network).
+    // I would like to understand this more: I am still seeing deadlocks when this drops below its current
+    // delay, but these maybe related to a different issue, as alluded to in
+    // https://github.com/mozilla/uniffi-rs/pull/1901
+    setTimeout(() => resolve(pollResult), 20);
+  }
 };
 
 // For testing only.