fix: only marshal promise resolution when created on the runtime loop

The promise proxy captures the run loop at construction and, when a
promise resolves on a different thread, marshals the resolution back to
that captured run loop. When a promise is constructed while native code
runs JS on a background thread, the captured run loop belongs to that
background thread, which is dormant once the call returns. A resolution
that arrives later on the runtime loop (for example from a setTimeout)
is scheduled onto the dormant run loop and never runs, so the promise
hangs forever.

Only marshal the resolution when the promise was created on the runtime
loop, which is always being pumped. A promise created on any other
thread resolves inline on whichever thread settles it. The same gate is
applied to the get trap on the returned promise.

Fixes #330

Author: adrian-niculescu

NativeScript/runtime/PromiseProxy.cpp

@@ -1,19 +1,36 @@
 #include "PromiseProxy.h"
+
+#include <CoreFoundation/CoreFoundation.h>
+
 #include "Helpers.h"
+#include "Runtime.h"
 
 using namespace v8;
 
 namespace tns {
 
+// Reports whether the calling thread runs the isolate's runtime loop. That loop
+// is where timers fire and is always being pumped, so a promise resolution is
+// marshaled back to its creating thread only when that thread is the runtime
+// loop; a promise created elsewhere settles on whichever thread resolves it.
+static void IsRuntimeRunloopCallback(const FunctionCallbackInfo<Value>& args) {
+    Runtime* runtime = Runtime::GetRuntime(args.GetIsolate());
+    bool isRuntimeLoop = runtime != nullptr && CFRunLoopGetCurrent() == runtime->RuntimeLoop();
+    args.GetReturnValue().Set(isRuntimeLoop);
+}
+
 void PromiseProxy::Init(v8::Local<v8::Context> context) {
     std::string source = R"(
-        // Ensure that Promise callbacks are executed on the
-        // same thread on which they were created
-        (() => {
+        // Run a Promise's callbacks on the thread that created it, but only when
+        // that thread is the runtime loop. A Promise created on a background
+        // thread settles on whichever thread resolves it, because the background
+        // run loop may be dormant and marshaling a resolution to it would hang.
+        (function(isRuntimeRunloop) {
             global.Promise = new Proxy(global.Promise, {
                 construct: function(target, args) {
                     let origFunc = args[0];
                     let runloop = CFRunLoopGetCurrent();
+                    let originIsRuntimeLoop = isRuntimeRunloop();
 
                     let promise = new target(function(resolve, reject) {
                         function isFulfilled() {
@@ -29,7 +46,7 @@ void PromiseProxy::Init(v8::Local<v8::Context> context) {
                                 return;
                             }
                             const resolveCall = resolve.bind(this, value);
-                            if (runloop === CFRunLoopGetCurrent()) {
+                            if (!originIsRuntimeLoop || runloop === CFRunLoopGetCurrent()) {
                                 markFulfilled();
                                 resolveCall();
                             } else {
@@ -42,7 +59,7 @@ void PromiseProxy::Init(v8::Local<v8::Context> context) {
                                 return;
                             }
                             const rejectCall = reject.bind(this, reason);
-                            if (runloop === CFRunLoopGetCurrent()) {
+                            if (!originIsRuntimeLoop || runloop === CFRunLoopGetCurrent()) {
                                 markFulfilled();
                                 rejectCall();
                             } else {
@@ -60,7 +77,7 @@ void PromiseProxy::Init(v8::Local<v8::Context> context) {
                                 return orig.bind(target);
                             }
                             return typeof orig === 'function' ? function(x) {
-                                if (runloop === CFRunLoopGetCurrent()) {
+                                if (!originIsRuntimeLoop || runloop === CFRunLoopGetCurrent()) {
                                     orig.bind(target, x)();
                                     return target;
                                 }
@@ -72,7 +89,7 @@ void PromiseProxy::Init(v8::Local<v8::Context> context) {
                     });
                 }
             });
-        })();
+        })
     )";
 
     Isolate* isolate = context->GetIsolate();
@@ -83,6 +100,17 @@ void PromiseProxy::Init(v8::Local<v8::Context> context) {
 
     Local<Value> result;
     success = script->Run(context).ToLocal(&result);
+    tns::Assert(success && result->IsFunction(), isolate);
+
+    Local<v8::Function> installProxy = result.As<v8::Function>();
+
+    Local<v8::Function> isRuntimeRunloop;
+    success = v8::Function::New(context, IsRuntimeRunloopCallback).ToLocal(&isRuntimeRunloop);
+    tns::Assert(success, isolate);
+
+    Local<Value> installArgs[] = { isRuntimeRunloop };
+    Local<Value> installResult;
+    success = installProxy->Call(context, context->Global(), 1, installArgs).ToLocal(&installResult);
     tns::Assert(success, isolate);
 }
 

TestRunner/app/tests/Promises.js

@@ -130,4 +130,21 @@ describe("Promise scheduling", function () {
             done();
         });
     });
+
+    it("the 'then' callback runs for a Promise created on a background thread and resolved on the runtime loop", done => {
+        // https://github.com/NativeScript/ios/issues/330
+        // The promise is constructed on a background dispatch queue whose run
+        // loop is dormant, then resolve() is invoked on the runtime loop. The
+        // resolution must run there instead of being marshaled back to the
+        // parked background loop, otherwise the promise never settles.
+        const backgroundQueue = dispatch_get_global_queue(qos_class_t.QOS_CLASS_DEFAULT, 0);
+        dispatch_async(backgroundQueue, () => {
+            new Promise(resolve => {
+                NSOperationQueue.mainQueue.addOperationWithBlock(() => resolve("settled"));
+            }).then(value => {
+                expect(value).toBe("settled");
+                done();
+            });
+        });
+    });
 });