Skip to content

Commit

Permalink
Add JS promise test, optimize V8/QuickJs event loop handling (#110)
Browse files Browse the repository at this point in the history
  • Loading branch information
LanderlYoung authored Sep 23, 2024
1 parent 4532e91 commit c3c1541
Show file tree
Hide file tree
Showing 10 changed files with 73 additions and 29 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/unit_tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -265,4 +265,4 @@ jobs:
cd build
# exclude failed tests
# --no-experimental-fetch config from https://github.com/emscripten-core/emscripten/issues/16915
node --no-experimental-fetch UnitTests.js '--gtest_filter=-ThreadPool.*:EngineScopeTest.ExitEngine:EngineScopeTest.TwoThreads:EngineScopeTest.ThreadLocal:MessageQueue.Interrupt:MessageQueue.Shutdown:MessageQueue.ShutdownNow:MessageQueue.FullAndPostInsideLoopQueue:ReferenceTest.WeakGc:ReferenceTest.WeakGc:ReferenceTest.GlobalNotClear:ReferenceTest.GlobalOnEngineDestroy:ReferenceTest.WeakOnEngineDestroy:ReferenceTest.WeakNotClrear:ManagedObjectTest.EngineDispose:ManagedObjectTest.FunctionCallback:PressureTest.All:EngineTest.JsPromiseTest:ShowCaseTest.SetTimeout'
node --no-experimental-fetch UnitTests.js '--gtest_filter=-ThreadPool.*:EngineScopeTest.ExitEngine:EngineScopeTest.TwoThreads:EngineScopeTest.ThreadLocal:MessageQueue.Interrupt:MessageQueue.Shutdown:MessageQueue.ShutdownNow:MessageQueue.FullAndPostInsideLoopQueue:ReferenceTest.WeakGc:ReferenceTest.WeakGc:ReferenceTest.GlobalNotClear:ReferenceTest.GlobalOnEngineDestroy:ReferenceTest.WeakOnEngineDestroy:ReferenceTest.WeakNotClrear:ManagedObjectTest.EngineDispose:ManagedObjectTest.FunctionCallback:PressureTest.All:EngineTest.JsPromiseTest:EngineTest.JsPromiseTest2:ShowCaseTest.SetTimeout'
2 changes: 1 addition & 1 deletion VERSION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
3.6.0
3.6.1
11 changes: 5 additions & 6 deletions backend/QuickJs/QjsEngine.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -203,9 +204,7 @@ void QjsEngine::scheduleTick() {
}
eng->tickScheduled_ = false;
},
[](auto& m) {

});
nullptr);
tick.ptr0 = this;
tick.tag = this;
queue_->postMessage(tick);
Expand Down Expand Up @@ -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);
}
Expand Down
2 changes: 1 addition & 1 deletion backend/QuickJs/QjsEngine.h
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ class QjsEngine : public ScriptEngine {
/**
* similar to js_std_loop
*/
void scheduleTick();
void triggerTick();

void extendLifeTimeToNextLoop(JSValue value);

Expand Down
2 changes: 1 addition & 1 deletion backend/QuickJs/QjsLocalReference.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
1 change: 1 addition & 0 deletions backend/QuickJs/QjsScope.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ EngineScopeImpl::EngineScopeImpl(QjsEngine &current, QjsEngine *prev)
}

EngineScopeImpl::~EngineScopeImpl() {
current_->triggerTick();
current_->runtimeLock_.unlock();
if (previous_) {
previous_->runtimeLock_.lock();
Expand Down
2 changes: 1 addition & 1 deletion backend/QuickJs/QjsValue.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
24 changes: 19 additions & 5 deletions backend/V8/V8Platform.cc
Original file line number Diff line number Diff line change
Expand Up @@ -102,11 +102,25 @@ 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));
});
if (engine_->isDestroying()) {
return;
}

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_;
Expand Down
4 changes: 2 additions & 2 deletions src/version.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,10 @@

// ScriptX version config files
// auto generated from the file VERSION
#define SCRIPTX_VERSION_STRING "3.6.0"
#define SCRIPTX_VERSION_STRING "3.6.1"
#define SCRIPTX_VERSION_MAJOR 3
#define SCRIPTX_VERSION_MINOR 6
#define SCRIPTX_VERSION_PATCH 0
#define SCRIPTX_VERSION_PATCH 1

namespace script {

Expand Down
52 changes: 41 additions & 11 deletions test/src/EngineTest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -135,26 +135,56 @@ TEST_F(EngineTest, LuaBuiltIns) {

#ifdef SCRIPTX_LANG_JAVASCRIPT
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;
{
EngineScope scope(engine);
auto setValue = Function::newFunction([&value](int val) {
// force new line
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);
});
});
)");
}

engine->messageQueue()->shutdown(true);
engine->messageQueue()->loopQueue(utils::MessageQueue::LoopType::kLoopAndWait);
EXPECT_EQ(value, 1);
}

TEST_F(EngineTest, JsPromiseTest2) {
int value = -1;
{
EngineScope scope(engine);
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(); // throws in promise callback
setValue(1);
});
)");
}

engine->messageQueue()->shutdown(true);
engine->messageQueue()->loopQueue(utils::MessageQueue::LoopType::kLoopAndWait);
EXPECT_EQ(value, 0);
}
#endif

} // namespace script::test

0 comments on commit c3c1541

Please sign in to comment.