chakrashim: use microtask queue to run Promise

Previously we enqueue Promise callbacks with "process.nextTick". This turns out incompatible with node/v8 and causing execution order differences, breaking some Promise uses. This change implements a micro task queue in each ContextShim and uses it to run Promise callbacks (read: nodejs/node-v0.x-archive#8325). PR-URL: #45 Reviewed-By: Sandeep Agarwal <Agarwal.Sandeep@microsoft.com>
nodejs · Mar 28, 2016 · b838007 · b838007
1 parent 2723d94
commit b838007
Show file tree

Hide file tree

Showing 6 changed files with 34 additions and 17 deletions.
diff --git a/deps/chakrashim/lib/chakra_shim.js b/deps/chakrashim/lib/chakra_shim.js
@@ -382,6 +382,9 @@
     };
   }
 
+  // Simulate v8 micro tasks queue
+  var microTasks = [];
+
   function patchUtils(utils) {
     var isUintRegex = /^(0|[1-9]\\d*)$/;
 
@@ -513,6 +516,12 @@
       return Symbol_for(key);
     };
     utils.ensureDebug = ensureDebug;
+    utils.enqueueMicrotask = function(task) {
+      microTasks.push(task);
+    };
+    utils.dequeueMicrotask = function(task) {
+      return microTasks.shift();
+    };
   }
 
   patchErrorTypes();

diff --git a/deps/chakrashim/src/jsrtcachedpropertyidref.inc b/deps/chakrashim/src/jsrtcachedpropertyidref.inc
@@ -70,6 +70,8 @@ DEF(getStackTrace)
 DEF(getSymbolKeyFor)
 DEF(getSymbolFor)
 DEF(ensureDebug)
+DEF(enqueueMicrotask)
+DEF(dequeueMicrotask)
 DEF(getFunctionName)
 DEF(getFileName)
 DEF(getColumnNumber)

diff --git a/deps/chakrashim/src/jsrtcontextshim.cc b/deps/chakrashim/src/jsrtcontextshim.cc
@@ -77,7 +77,9 @@ ContextShim::ContextShim(IsolateShim * isolateShim,
       getStackTraceFunction(JS_INVALID_REFERENCE),
       getSymbolKeyForFunction(JS_INVALID_REFERENCE),
       getSymbolForFunction(JS_INVALID_REFERENCE),
-      ensureDebugFunction(JS_INVALID_REFERENCE) {
+      ensureDebugFunction(JS_INVALID_REFERENCE),
+      enqueueMicrotaskFunction(JS_INVALID_REFERENCE),
+      dequeueMicrotaskFunction(JS_INVALID_REFERENCE) {
   memset(globalConstructor, 0, sizeof(globalConstructor));
   memset(globalPrototypeFunction, 0, sizeof(globalPrototypeFunction));
 }
@@ -458,22 +460,21 @@ void ContextShim::SetAlignedPointerInEmbedderData(int index, void * value) {
   }
 }
 
-JsValueRef ContextShim::GetPromiseContinuationFunction() {
-  if (promiseContinuationFunction == JS_INVALID_REFERENCE) {
-    JsValueRef process;
-    if (jsrt::GetPropertyOfGlobal(L"process", &process) != JsNoError) {
-      return JS_INVALID_REFERENCE;
-    }
+void ContextShim::RunMicrotasks() {
+  JsValueRef dequeueMicrotaskFunction = GetdequeueMicrotaskFunction();
 
-    JsValueRef nextTick;
-    if (jsrt::GetProperty(process, L"nextTick", &nextTick) != JsNoError) {
-      return JS_INVALID_REFERENCE;
+  for (;;) {
+    JsValueRef task;
+    if (jsrt::CallFunction(dequeueMicrotaskFunction, &task) != JsNoError ||
+      reinterpret_cast<v8::Value*>(task)->IsUndefined()) {
+      break;
     }
 
-    // CHAKRA-REVIEW: Do we need to root this?
-    promiseContinuationFunction = nextTick;  // save in context data
+    JsValueRef notUsed;
+    if (jsrt::CallFunction(task, &notUsed) != JsNoError) {
+      JsGetAndClearException(&notUsed); // swallow any exception from task
+    }
   }
-  return promiseContinuationFunction;
 }
 
 JsValueRef ContextShim::GetUndefined() {
@@ -580,6 +581,8 @@ CHAKRASHIM_FUNCTION_GETTER(getStackTrace)
 CHAKRASHIM_FUNCTION_GETTER(getSymbolKeyFor)
 CHAKRASHIM_FUNCTION_GETTER(getSymbolFor)
 CHAKRASHIM_FUNCTION_GETTER(ensureDebug)
+CHAKRASHIM_FUNCTION_GETTER(enqueueMicrotask);
+CHAKRASHIM_FUNCTION_GETTER(dequeueMicrotask);
 
 #define DEF_IS_TYPE(F) CHAKRASHIM_FUNCTION_GETTER(F)
 #include "jsrtcachedpropertyidref.inc"

diff --git a/deps/chakrashim/src/jsrtcontextshim.h b/deps/chakrashim/src/jsrtcontextshim.h
@@ -57,7 +57,6 @@ class ContextShim {
   IsolateShim * GetIsolateShim();
   JsContextRef GetContextRef();
 
-  JsValueRef GetPromiseContinuationFunction();
   JsValueRef GetTrue();
   JsValueRef GetFalse();
   JsValueRef GetUndefined();
@@ -81,6 +80,7 @@ class ContextShim {
 
   void * GetAlignedPointerFromEmbedderData(int index);
   void SetAlignedPointerInEmbedderData(int index, void * value);
+  void RunMicrotasks();
 
   static ContextShim * GetCurrent();
 
@@ -147,6 +147,8 @@ JsValueRef F##Function; \
   DECLARE_CHAKRASHIM_FUNCTION_GETTER(getSymbolKeyFor);
   DECLARE_CHAKRASHIM_FUNCTION_GETTER(getSymbolFor);
   DECLARE_CHAKRASHIM_FUNCTION_GETTER(ensureDebug);
+  DECLARE_CHAKRASHIM_FUNCTION_GETTER(enqueueMicrotask);
+  DECLARE_CHAKRASHIM_FUNCTION_GETTER(dequeueMicrotask);
 
 #define DEF_IS_TYPE(F) DECLARE_CHAKRASHIM_FUNCTION_GETTER(F)
 #include "jsrtcachedpropertyidref.inc"

diff --git a/deps/chakrashim/src/jsrtpromise.cc b/deps/chakrashim/src/jsrtpromise.cc
@@ -25,11 +25,11 @@ namespace jsrt {
 
 static void CALLBACK PromiseContinuationCallback(JsValueRef task,
                                                  void *callbackState) {
-  JsValueRef promiseContinuationFunction =
-    ContextShim::GetCurrent()->GetPromiseContinuationFunction();
+  JsValueRef enqueueMicrotaskFunction =
+    ContextShim::GetCurrent()->GetenqueueMicrotaskFunction();
 
   JsValueRef result;
-  jsrt::CallFunction(promiseContinuationFunction, task, &result);
+  jsrt::CallFunction(enqueueMicrotaskFunction, task, &result);
 }
 
 JsErrorCode InitializePromise() {

diff --git a/deps/chakrashim/src/v8isolate.cc b/deps/chakrashim/src/v8isolate.cc
@@ -104,6 +104,7 @@ void Isolate::SetJitCodeEventHandler(JitCodeEventOptions options,
 }
 
 void Isolate::RunMicrotasks() {
+  jsrt::ContextShim::GetCurrent()->RunMicrotasks();
 }
 
 void Isolate::SetAutorunMicrotasks(bool autorun) {