Tencent/ScriptX

Exception when using promises with QuickJs backend

Archie3d opened this issue · 9 comments

Hi,
When running the following code with QuickJs backend an exception is thrown

const promise = new Promise((resolve, reject) => {
    setTimeout(() => { resolve('Ok') }, 300);
});

promise.then(x => { console.log(x); });  // Exception gets thrown here

The exception is about the missing engine scope, thrown from QjsEngine::newPrototype newRawFunction lambda. This is because when executing the promise_reaction_job (that is the .then() part) and eventually the lambda function created in QjsEngine::newPrototype, the engine scope is not set (the promise resolution is called on the message loop) so the args.thiz() in newRawFunction throws an exception.

It seems this can be solved by creating a scope in there:

// QjsEngine.hpp
// QjsEngine::newPrototype
    auto fun = newRawFunction(
        this, const_cast<FuncDef*>(&f), definePtr,
        [](const Arguments& args, void* data1, void* data2, bool) {
          EngineScope scope(args.engine());  // <---------------------------- *HERE*
          auto ptr = static_cast<InstanceClassOpaque*>(
              JS_GetOpaque(qjs_interop::peekLocal(args.thiz()), kInstanceClassId));
          if (ptr == nullptr || ptr->classDefine != data2) {
            throw Exception(u8"call function on wrong receiver");
          }
          auto f = static_cast<FuncDef*>(data1);
          Tracer tracer(args.engine(), f->traceName);
          return (f->callback)(static_cast<T*>(ptr->scriptClassPolymorphicPointer), args);
        });

Nice catch. Thanks for the report, will check this out later.

The js code looks just ok. Just to be clear, is this bug produced by calling "native method" in the then block?

This seems to be related to the setTimeout implementation.

You must implement your own setTimeout function, like this:

Function::newFunction([](args) {
  messageQueue.sendMessage([]() {
    EngineScope scope(args.engine());
    args.callback.call();
  }
});

update: added reference implementation #67

Hi, thanks for your response. I do have setTimeout implementation which sets the scope and works fine. But the exception is not happening on the setTimeout() callback though, but when calling then() method on returned promise.

em... a little bit strange, will check this out later.

Still works as expected.
Code: f6d813b
Run: https://github.com/Tencent/ScriptX/runs/4472930615?check_suite_focus=true

Did I miss something? Maybe a full code to reproduce is needed.

Hi, this is actually not related to setTimeout. Here is the code that fails for me and it does not use setTimeout:

const promise = new Promise((resolve, reject) => {
    resolve('Ok');
} );

// Exception gets thrown here
promise.then(x => { console.log(x); });

The exception actually happens when QuickJs attemps to call the lambda from .then().
Could you adjust the unit test to check the x => { console.log(x); } gets actually called in your case?

Here is the stack trace for the exception:

Sandbox.exe!script::EngineScope::ensureEngineScope(void * engine) Line 71 (c:\prj\sandbox\ScriptX\src\Scope.cc:71)
Sandbox.exe!script::EngineScope::currentEngineCheckedAs<script::qjs_backend::QjsEngine>() Line 113 (c:\prj\sandbox\ScriptX\src\Scope.h:113)
Sandbox.exe!script::qjs_backend::currentEngine() Line 30 (c:\prj\sandbox\ScriptX\backend\QuickJs\QjsHelper.cc:30)
Sandbox.exe!script::qjs_backend::currentContext() Line 26 (c:\prj\sandbox\ScriptX\backend\QuickJs\QjsHelper.cc:26)
Sandbox.exe!script::qjs_backend::dupValue(unsigned __int64 val, JSContext * context) Line 54 (c:\prj\sandbox\ScriptX\backend\QuickJs\QjsHelper.cc:54)
Sandbox.exe!script::Local<script::Value>::asObject() Line 201 (c:\prj\sandbox\ScriptX\backend\QuickJs\QjsLocalReference.cc:201)
Sandbox.exe!script::Arguments::thiz() Line 27 (c:\prj\sandbox\ScriptX\backend\QuickJs\QjsNative.cc:27)
Sandbox.exe!`script::qjs_backend::QjsEngine::newPrototype<ConsoleWrapper>'::`6'::<lambda_1>::operator()(const script::Arguments & args, void * data1, void * data2, bool __formal) Line 134 (c:\prj\sandbox\ScriptX\backend\QuickJs\QjsEngine.hpp:134)
Sandbox.exe!_Closure_wrapper_098eb497_2::<lambda_invoker_cdecl>(const script::Arguments & __p1, void * __p2, void * __p3, bool __p4) Line 142 (c:\prj\sandbox\ScriptX\backend\QuickJs\QjsEngine.hpp:142)
Sandbox.exe!`script::qjs_backend::newRawFunction'::`2'::<lambda_2>::operator()(JSContext * ctx, unsigned __int64 this_val, int argc, unsigned __int64 * argv, int magic, unsigned __int64 * func_data) Line 104 (c:\prj\sandbox\ScriptX\backend\QuickJs\QjsHelper.cc:104)
Sandbox.exe!_Closure_wrapper_28154f09_1::<lambda_invoker_cdecl>(JSContext * __p1, unsigned __int64 __p2, int __p3, unsigned __int64 * __p4, int __p5, unsigned __int64 * __p6) Line 110 (c:\prj\sandbox\ScriptX\backend\QuickJs\QjsHelper.cc:110)
Sandbox.exe!js_c_function_data_call(JSContext * ctx, unsigned __int64 func_obj, unsigned __int64 this_val, int argc, unsigned __int64 * argv, int flags) Line 5224 (c:\prj\sandbox\quickjs\quickjs.c:5224)
Sandbox.exe!JS_CallInternal(JSContext * caller_ctx, unsigned __int64 func_obj, unsigned __int64 this_obj, unsigned __int64 new_target, int argc, unsigned __int64 * argv, int flags) Line 16525 (c:\prj\sandbox\quickjs\quickjs.c:16525)
Sandbox.exe!JS_CallInternal(JSContext * caller_ctx, unsigned __int64 func_obj, unsigned __int64 this_obj, unsigned __int64 new_target, int argc, unsigned __int64 * argv, int flags) Line 16932 (c:\prj\sandbox\quickjs\quickjs.c:16932)
Sandbox.exe!JS_Call(JSContext * ctx, unsigned __int64 func_obj, unsigned __int64 this_obj, int argc, unsigned __int64 * argv) Line 18982 (c:\prj\sandbox\quickjs\quickjs.c:18982)
Sandbox.exe!promise_reaction_job(JSContext * ctx, int argc, unsigned __int64 * argv) Line 46798 (c:\prj\sandbox\quickjs\quickjs.c:46798)
Sandbox.exe!JS_ExecutePendingJob(JSRuntime * rt, JSContext * * pctx) Line 1906 (c:\prj\sandbox\quickjs\quickjs.c:1906)
Sandbox.exe!`script::qjs_backend::QjsEngine::scheduleTick'::`5'::<lambda_1>::operator()<script::utils::Message>(script::utils::Message & m) Line 199 (c:\prj\sandbox\ScriptX\backend\QuickJs\QjsEngine.cc:199)
Sandbox.exe!`script::qjs_backend::QjsEngine::scheduleTick'::`5'::<lambda_1>::<lambda_invoker_cdecl><script::utils::Message>(script::utils::Message & m) Line 202 (c:\prj\sandbox\ScriptX\backend\QuickJs\QjsEngine.cc:202)
Sandbox.exe!script::utils::Message::handle() Line 90 (c:\prj\sandbox\ScriptX\src\utils\MessageQueue.cc:90)
Sandbox.exe!script::utils::MessageQueue::processMessage(script::utils::Message * message) Line 401 (c:\prj\sandbox\ScriptX\src\utils\MessageQueue.cc:401)
Sandbox.exe!script::utils::MessageQueue::loopQueue(script::utils::MessageQueue::LoopType loopType) Line 391 (c:\prj\sandbox\ScriptX\src\utils\MessageQueue.cc:391)

Ahh... the stack helps.
fixed b13a711
please try this branch

Excellent, that works, thank you!