From 55a026b377d79fafbd171aad0508b7a2553dd530 Mon Sep 17 00:00:00 2001 From: landerlyoung Date: Thu, 9 Dec 2021 14:44:47 +0800 Subject: [PATCH 1/4] add show case for setTimeout, reference implmentation --- .github/workflows/unit_tests.yml | 2 +- backend/WebAssembly/WasmEngine.cc | 4 +- backend/WebAssembly/WasmEngine.h | 4 +- docs/en/WebAssembly.md | 2 +- docs/zh/WebAssembly.md | 2 +- src/utils/MessageQueue.h | 2 + test/CMakeLists.txt | 10 ++-- test/src/EngineTest.cc | 7 +-- test/src/ShowCaseTest.cc | 95 +++++++++++++++++++++++++++++++ test/src/gtest_main.cc | 2 +- 10 files changed, 112 insertions(+), 18 deletions(-) create mode 100644 test/src/ShowCaseTest.cc diff --git a/.github/workflows/unit_tests.yml b/.github/workflows/unit_tests.yml index 8174410d..d445d372 100644 --- a/.github/workflows/unit_tests.yml +++ b/.github/workflows/unit_tests.yml @@ -236,4 +236,4 @@ jobs: run: | cd build # exclude failed tests - node 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' + node 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' diff --git a/backend/WebAssembly/WasmEngine.cc b/backend/WebAssembly/WasmEngine.cc index c88ffd9a..c52d446b 100644 --- a/backend/WebAssembly/WasmEngine.cc +++ b/backend/WebAssembly/WasmEngine.cc @@ -26,8 +26,6 @@ namespace script::wasm_backend { -std::thread::id WasmEngine::engineThreadId_{}; - WasmEngine::WasmEngine() { engineThreadId_ = std::this_thread::get_id(); } WasmEngine* WasmEngine::instance() { @@ -49,7 +47,7 @@ void WasmEngine::destroy() { bool WasmEngine::isDestroying() const { return false; } -void WasmEngine::unitTestResetRetistry() { +void WasmEngine::unitTestResetRegistry() { classDefineRegistry_.clear(); ScriptEngine::classDefineRegistry_.clear(); } diff --git a/backend/WebAssembly/WasmEngine.h b/backend/WebAssembly/WasmEngine.h index 0d0ec207..b8d629f2 100644 --- a/backend/WebAssembly/WasmEngine.h +++ b/backend/WebAssembly/WasmEngine.h @@ -39,12 +39,12 @@ class WasmEngine : public ScriptEngine { std::shared_ptr messageQueue_ = std::make_shared(); bool ignoreDestroyCall_ = false; - static std::thread::id engineThreadId_; + std::thread::id engineThreadId_ = {}; WasmEngine(); // for unit-test only - void unitTestResetRetistry(); + void unitTestResetRegistry(); public: static WasmEngine* instance(); diff --git a/docs/en/WebAssembly.md b/docs/en/WebAssembly.md index 13de1080..d020afb8 100644 --- a/docs/en/WebAssembly.md +++ b/docs/en/WebAssembly.md @@ -71,7 +71,7 @@ The memory can only be managed manually. There are two main aspects related to S In V8 and JSCore, depending on the finalize callback provided by the engine, the automatic release of the bound class is realized, but in WASM it is not possible, and the user can only release it by himself. ScriptX provides auxiliary methods in the JS global scope. -Give a chestnut: +For example: ```C++ static ClassDefine test = diff --git a/docs/zh/WebAssembly.md b/docs/zh/WebAssembly.md index b41debbe..7bc896f8 100644 --- a/docs/zh/WebAssembly.md +++ b/docs/zh/WebAssembly.md @@ -68,7 +68,7 @@ Wasm没有GC，JS也没有finalize回调。。。所以就很坑爹 2. 绑定类的内存释放 ### 绑定类的内存释放 -在V8和JSCore中，依赖引擎提供的finalize回调，实现了绑定类的自动释放，但是在WASM中就搞不定了，使用者只能自己释放只。ScriptX在JS全局提供了辅助方法。 +在V8和JSCore中，依赖引擎提供的finalize回调，实现了绑定类的自动释放，但是在WASM中就搞不定了，使用者只能自己释放之。ScriptX在JS全局提供了辅助方法。 举个栗子： diff --git a/src/utils/MessageQueue.h b/src/utils/MessageQueue.h index 7f6adc69..73e823f9 100644 --- a/src/utils/MessageQueue.h +++ b/src/utils/MessageQueue.h @@ -50,6 +50,8 @@ struct alignas(std::max_align_t) ArbitraryData { ArbitraryData& operator=(const ArbitraryData&); + SCRIPTX_DISALLOW_MOVE(ArbitraryData); + ~ArbitraryData() = default; }; diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 694ce856..e277e827 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -13,8 +13,6 @@ add_executable(UnitTests) include(cmake/TestEnv.cmake) if (CMAKE_CXX_COMPILER_ID MATCHES "Clang" OR CMAKE_CXX_COMPILER_ID STREQUAL "GNU") - string(APPEND CMAKE_CXX_FLAGS_RELEASE " -O3 -flto") - if (TEST_FLAG_ENABLE_ASAN) # macOS doesn't support -fsanitize=memory string(APPEND UNIT_TEST_ASAN_FLAGS @@ -39,10 +37,10 @@ if (CMAKE_CXX_COMPILER_ID MATCHES "Clang" OR CMAKE_CXX_COMPILER_ID STREQUAL "GNU # devops support gcov only if (CMAKE_CXX_COMPILER_ID MATCHES "Clang") # for clang - string(APPEND CMAKE_CXX_FLAGS " --coverage -g3") + string(APPEND CMAKE_CXX_FLAGS " --coverage") else () # for gcc - string(APPEND CMAKE_CXX_FLAGS " -fprofile-arcs -ftest-coverage -g3") + string(APPEND CMAKE_CXX_FLAGS " -fprofile-arcs -ftest-coverage") endif() endif () endif () @@ -73,7 +71,9 @@ target_sources(UnitTests PRIVATE src/InteroperateTest.cc src/ExceptionTest.cc src/PressureTest.cc - src/EngineTest.cc) + src/EngineTest.cc + src/ShowCaseTest.cc + ) ######## ScriptX config ########## diff --git a/test/src/EngineTest.cc b/test/src/EngineTest.cc index 8d409361..ed73711d 100644 --- a/test/src/EngineTest.cc +++ b/test/src/EngineTest.cc @@ -143,12 +143,11 @@ TEST_F(EngineTest, JsPromiseTest) { engine->eval( u8R"( const promise = new Promise((resolve, reject) => { - resolve('Ok'); + resolve(1); }); - promise.then(x => { - console.log(x); - setValue(1); + promise.then(num => { + setValue(num); }); )"); diff --git a/test/src/ShowCaseTest.cc b/test/src/ShowCaseTest.cc new file mode 100644 index 00000000..b122fb83 --- /dev/null +++ b/test/src/ShowCaseTest.cc @@ -0,0 +1,95 @@ +/* + * Tencent is pleased to support the open source community by making ScriptX available. + * Copyright (C) 2021 THL A29 Limited, a Tencent company. All rights reserved. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "test.h" + +namespace script::test { + +DEFINE_ENGINE_TEST(ShowCaseTest); + +/** + * show how to implement a `setTimeout(function, delayMs)` + */ +namespace { + +void defineSetTimeout(ScriptEngine* engine) { + EngineScope scope(engine); + auto setTimeOut = Function::newFunction([](const Arguments& args) -> Local { + // make a global ref, we are going to pass it beyond stack-frame lifecycle + auto callback = Global(args[0].asFunction()); + auto delayMs = args[1].asNumber().toInt64(); + auto engine = args.engine(); + + auto runner = [fun = std::move(callback), engine]() { + // make a scope + EngineScope scope(engine); + try { + // run the callback function + fun.get().call(); + } catch (const Exception& e) { + Logger() << "error in setTimeout:" << e; + } + }; + using RunnerType = decltype(runner); + + auto&& queue = args.engine()->messageQueue(); + auto msg = queue->obtainInplaceMessage( + [](utils::InplaceMessage& msg) { msg.getObject()(); }); + msg->inplaceObject(std::move(runner)); + queue->postMessage(msg, std::chrono::milliseconds(delayMs)); + + return {}; + }); + + engine->set("test_setTimeout", setTimeOut); +} + +} // namespace + +TEST_F(ShowCaseTest, SetTimeout) { + defineSetTimeout(engine); + { + EngineScope scope(engine); + int mark = 0; + + engine->set("setMark", Function::newFunction([&mark](int v) { mark = v; })); + + engine->eval(TS().js(u8R"( + test_setTimeout(() => { + setMark(1); + test_setTimeout(() => setMark(2), 0); + }, 0); + )") + .lua(u8R"( + test_setTimeout( + function() + setMark(1); + test_setTimeout(function() setMark(2) end, 0); + end, + 0); + )") + .select()); + auto&& queue = engine->messageQueue(); + ASSERT_EQ(mark, 0); + queue->loopQueue(utils::MessageQueue::LoopType::kLoopOnce); + ASSERT_EQ(mark, 1); + queue->loopQueue(utils::MessageQueue::LoopType::kLoopOnce); + ASSERT_EQ(mark, 2); + } +} + +} // namespace script::test diff --git a/test/src/gtest_main.cc b/test/src/gtest_main.cc index ec87ca64..e9a0258b 100644 --- a/test/src/gtest_main.cc +++ b/test/src/gtest_main.cc @@ -53,7 +53,7 @@ void ScriptXTestFixture::SetUp() { #ifdef SCRIPTX_BACKEND_WEBASSEMBLY auto eng = script::ScriptEngineImpl::instance(); script::ScriptEngineImpl::ignoreDestroyCall(); - eng->unitTestResetRetistry(); + eng->unitTestResetRegistry(); engine = eng; #else engine = new script::ScriptEngineImpl(); From 69135add2f3bd26f0d6971e115d18421a7cd5812 Mon Sep 17 00:00:00 2001 From: landerlyoung Date: Sat, 11 Dec 2021 20:19:55 +0800 Subject: [PATCH 2/4] update emscripten version --- .github/workflows/unit_tests.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/unit_tests.yml b/.github/workflows/unit_tests.yml index d445d372..ed38235f 100644 --- a/.github/workflows/unit_tests.yml +++ b/.github/workflows/unit_tests.yml @@ -212,10 +212,10 @@ jobs: build/ScriptXTestLibs build/googletest-src - name: Setup Emscripten - uses: mymindstorm/setup-emsdk@v8 + uses: mymindstorm/setup-emsdk@v11 with: - version: '2.0.5' - actions-cache-folder: 'emsdk-cache' + version: '3.0.0' + actions-cache-folder: 'emsdk-cache-3.0.0' - name: Setup Node.js uses: actions/setup-node@v1 with: From 542a3807ec1f777dc159089a1f09a58af079732a Mon Sep 17 00:00:00 2001 From: landerlyoung Date: Sun, 12 Dec 2021 19:02:29 +0800 Subject: [PATCH 3/4] use atomic_thread_fence to insert a memory barrier --- src/Utils.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Utils.cc b/src/Utils.cc index aeb74055..d075f7e9 100644 --- a/src/Utils.cc +++ b/src/Utils.cc @@ -25,7 +25,7 @@ Tracer::Delegate* Tracer::delegate_; void Tracer::setDelegate(Tracer::Delegate* d) { delegate_ = d; // use atomic to insert a memory barrier here. - std::atomic_int().store(0, std::memory_order_release); + std::atomic_thread_fence(std::memory_order_release); } Tracer::Tracer(ScriptEngine* engine, const char* traceName) noexcept : engine_(engine) { From caa1c16cca7fa1ed2acf499880652d1828e61f35 Mon Sep 17 00:00:00 2001 From: landerlyoung Date: Sun, 12 Dec 2021 19:25:18 +0800 Subject: [PATCH 4/4] bump version to 3.2.0 --- CHANGELOG.md | 3 +++ VERSION | 2 +- src/version.h | 4 ++-- 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 579cf043..551df6c8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,8 @@ --- +Version 3.2.0 (2021-12): +1. fix QuickJs memory leak, engine instance not deleted on destroy +2. fix QuickJs missing EngineScope when run micro task (Promise) Version 3.1.0 (2021-04): 1. add QuickJs backend diff --git a/VERSION b/VERSION index a0cd9f0c..a4f52a5d 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -3.1.0 \ No newline at end of file +3.2.0 \ No newline at end of file diff --git a/src/version.h b/src/version.h index f4b6553c..ed455cb9 100644 --- a/src/version.h +++ b/src/version.h @@ -19,9 +19,9 @@ // ScriptX version config files // auto generated from the file VERSION -#define SCRIPTX_VERSION_STRING "3.1.0" +#define SCRIPTX_VERSION_STRING "3.2.0" #define SCRIPTX_VERSION_MAJOR 3 -#define SCRIPTX_VERSION_MINOR 1 +#define SCRIPTX_VERSION_MINOR 2 #define SCRIPTX_VERSION_PATCH 0 namespace script {