add JS promise test, fix possible bugs in event loop handling

Author: LanderlYoung

backend/QuickJs/QjsEngine.cc

@@ -191,9 +191,10 @@ void QjsEngine::destroy() noexcept {
   delete this;
 }
 
-void QjsEngine::scheduleTick() {
+void QjsEngine::triggerTick() {
   bool no = false;
-  if (tickScheduled_.compare_exchange_strong(no, true)) {
+  if (!isDestroying() && JS_IsJobPending(runtime_) &&
+      tickScheduled_.compare_exchange_strong(no, true)) {
     utils::Message tick(
         [](auto& m) {
           auto eng = static_cast<QjsEngine*>(m.ptr0);
@@ -203,9 +204,7 @@ void QjsEngine::scheduleTick() {
           }
           eng->tickScheduled_ = false;
         },
-        [](auto& m) {
-
-        });
+        nullptr);
     tick.ptr0 = this;
     tick.tag = this;
     queue_->postMessage(tick);
@@ -263,7 +262,7 @@ Local<Value> QjsEngine::eval(const Local<String>& script, const Local<Value>& so
   }
   qjs_backend::checkException(ret);
 
-  scheduleTick();
+  triggerTick();
 
   return Local<Value>(ret);
 }

backend/QuickJs/QjsEngine.h

@@ -140,7 +140,7 @@ class QjsEngine : public ScriptEngine {
   /**
    * similar to js_std_loop
    */
-  void scheduleTick();
+  void triggerTick();
 
   void extendLifeTimeToNextLoop(JSValue value);
 

backend/QuickJs/QjsLocalReference.cc

@@ -332,7 +332,7 @@ Local<Value> Local<Function>::callImpl(const Local<Value>& thiz, size_t size,
         qjs_backend::checkException(ret);
       });
 
-  engine.scheduleTick();
+  engine.triggerTick();
   return qjs_interop::makeLocal<Value>(ret);
 }
 

backend/QuickJs/QjsScope.cc

@@ -30,6 +30,7 @@ EngineScopeImpl::EngineScopeImpl(QjsEngine &current, QjsEngine *prev)
 }
 
 EngineScopeImpl::~EngineScopeImpl() {
+  current_->triggerTick();
   current_->runtimeLock_.unlock();
   if (previous_) {
     previous_->runtimeLock_.lock();

backend/QuickJs/QjsValue.cc

@@ -47,7 +47,7 @@ Local<Object> Object::newObjectImpl(const Local<Value>& type, size_t size,
     qjs_backend::checkException(ret);
   });
 
-  engine.scheduleTick();
+  engine.triggerTick();
   return qjs_interop::makeLocal<Object>(ret);
 }
 

backend/V8/V8Platform.cc

@@ -102,11 +102,21 @@ class MessageQueueTaskRunner : public v8::TaskRunner {
 
  private:
   void scheduleTask(std::unique_ptr<v8::Task> task, double delay_in_seconds = 0) {
-    script::utils::Message s([](auto& msg) { static_cast<v8::Task*>(msg.ptr0)->Run(); },
-                             [](auto& msg) {
-                               using deleter = std::unique_ptr<v8::Task>::deleter_type;
-                               deleter{}(static_cast<v8::Task*>(msg.ptr0));
-                             });
+    script::utils::Message s(
+        [](auto& msg) {
+          auto engine = static_cast<V8Engine*>(msg.tag);
+          EngineScope scope(engine);
+          try {
+            static_cast<v8::Task*>(msg.ptr0)->Run();
+          } catch (const Exception& e) {
+            // this should not happen, all JS exceptions should be handled by V8
+            abort();
+          }
+        },
+        [](auto& msg) {
+          using deleter = std::unique_ptr<v8::Task>::deleter_type;
+          deleter{}(static_cast<v8::Task*>(msg.ptr0));
+        });
     s.name = "SchedulePump";
     s.ptr0 = task.release();
     s.tag = engine_;

test/src/EngineTest.cc

@@ -137,24 +137,57 @@ TEST_F(EngineTest, LuaBuiltIns) {
 TEST_F(EngineTest, JsPromiseTest) {
   EngineScope scope(engine);
 
-  int value = 0;
-  auto setValue = Function::newFunction([&value](int val) { value = val; });
-  engine->set("setValue", setValue);
-  engine->eval(
-      u8R"(
-        const promise = new Promise((resolve, reject) => {
+  int value = -1;
+  try {
+    auto setValue = Function::newFunction([&value](int val) { value = val; });
+    engine->set("setValue", setValue);
+    engine->eval(
+        u8R"(
+        new Promise((resolve, reject) => {
+            resolve(0);
+        }).then(() => {
+          setValue(0);
+          new Promise((resolve, reject) => {
             resolve(1);
-        });
-
-        promise.then(num => {
+          }).then(num => {
             setValue(num);
+          });
         });
     )");
+  } catch (const Exception& e) {
+    FAIL() << e;
+  }
 
   engine->messageQueue()->shutdown(true);
   engine->messageQueue()->loopQueue(utils::MessageQueue::LoopType::kLoopAndWait);
   EXPECT_EQ(value, 1);
 }
+
+TEST_F(EngineTest, JsPromiseTest2) {
+  EngineScope scope(engine);
+
+  int value = -1;
+  try {
+    auto setValue = Function::newFunction([&value](int val) { value = val; });
+    engine->set("setValue", setValue);
+    engine->eval(
+        u8R"(
+        new Promise((resolve, reject) => {
+            resolve(0);
+        }).then(() => {
+          setValue(0);
+          throw error();
+          setValue(1);
+        });
+    )");
+  } catch (const Exception& e) {
+    FAIL() << e;
+  }
+
+  engine->messageQueue()->shutdown(true);
+  engine->messageQueue()->loopQueue(utils::MessageQueue::LoopType::kLoopAndWait);
+  EXPECT_EQ(value, 0);
+}
 #endif
 
 }  // namespace script::test