From 8df655d0e914cb7798827478a9a214b868d03b0b Mon Sep 17 00:00:00 2001 From: qiuguohua Date: Sat, 8 Feb 2025 14:55:46 +0800 Subject: [PATCH 1/5] Fix restart crash on HarmonyOS NEXT platform when using JSVM. --- .../cocos/bindings/jswrapper/jsvm/Object.cpp | 42 ++++++-------- native/cocos/bindings/jswrapper/jsvm/Object.h | 6 -- .../bindings/jswrapper/jsvm/ScriptEngine.cpp | 57 ++++++++++--------- .../bindings/jswrapper/jsvm/ScriptEngine.h | 5 ++ native/cocos/engine/Engine.cpp | 4 ++ 5 files changed, 57 insertions(+), 57 deletions(-) diff --git a/native/cocos/bindings/jswrapper/jsvm/Object.cpp b/native/cocos/bindings/jswrapper/jsvm/Object.cpp index 25163543ae9..efbc39f984e 100644 --- a/native/cocos/bindings/jswrapper/jsvm/Object.cpp +++ b/native/cocos/bindings/jswrapper/jsvm/Object.cpp @@ -36,18 +36,17 @@ namespace se { std::unique_ptr> __objectMap; // Currently, the value `void*` is always nullptr -std::set Object::objBaseSet = {}; -bool Object::restarting = false; + Object::Object() {} Object::~Object() { - if(restarting) { - objBaseSet.insert(this); - } if (__objectMap) { __objectMap->erase(this); } + + delete _privateObject; + _privateObject = nullptr; } Object* Object::createObjectWithClass(Class* cls) { @@ -652,7 +651,6 @@ std::string Object::toString() const { } void Object::root() { - JSVM_Status status; if (_rootCount == 0) { uint32_t result = 0; _objRef.incRef(_env); @@ -661,7 +659,6 @@ void Object::root() { } void Object::unroot() { - JSVM_Status status; if (_rootCount > 0) { --_rootCount; if (_rootCount == 0) { @@ -704,37 +701,34 @@ void Object::weakCallback(JSVM_Env env, void* nativeObject, void* finalizeHint / } void* rawPtr = reinterpret_cast(finalizeHint)->_privateData; Object* seObj = reinterpret_cast(finalizeHint); - - auto it = objBaseSet.find(seObj); - if(it != objBaseSet.end()) { + if (seObj->_onCleaingPrivateData) { //called by cleanPrivateData, not release seObj; return; } - - if (seObj->_onCleaingPrivateData) { //called by cleanPrivateData, not release seObj; + if(!NativePtrToObjectMap::isValid()) { return; } if (seObj->_clearMappingInFinalizer && rawPtr != nullptr) { auto iter = NativePtrToObjectMap::find(rawPtr); if (iter != NativePtrToObjectMap::end()) { - if (seObj->_finalizeCb != nullptr) { - seObj->_finalizeCb(env, finalizeHint, finalizeHint); - } else { - assert(seObj->_getClass() != nullptr); - if (seObj->_getClass()->_getFinalizeFunction() != nullptr) { - seObj->_getClass()->_getFinalizeFunction()(env, finalizeHint, finalizeHint); - } - } - seObj->decRef(); NativePtrToObjectMap::erase(iter); } else { SE_LOGE("not find ptr in NativePtrToObjectMap"); } } + + if (seObj->_finalizeCb != nullptr) { + seObj->_finalizeCb(env, finalizeHint, finalizeHint); + } else { + assert(seObj->_getClass() != nullptr); + if (seObj->_getClass()->_getFinalizeFunction() != nullptr) { + seObj->_getClass()->_getFinalizeFunction()(env, finalizeHint, finalizeHint); + } + } + seObj->decRef(); } } void Object::setup() { - restarting = false; __objectMap = std::make_unique>(); } @@ -749,11 +743,11 @@ void Object::cleanup() { obj = e.second; if (obj->_finalizeCb != nullptr) { - obj->_finalizeCb(ScriptEngine::getEnv(), nativeObj, nullptr); + obj->_finalizeCb(ScriptEngine::getEnv(), obj, nullptr); } else { if (obj->_getClass() != nullptr) { if (obj->_getClass()->_getFinalizeFunction() != nullptr) { - obj->_getClass()->_getFinalizeFunction()(ScriptEngine::getEnv(), nativeObj, nullptr); + obj->_getClass()->_getFinalizeFunction()(ScriptEngine::getEnv(), obj, nullptr); } } } diff --git a/native/cocos/bindings/jswrapper/jsvm/Object.h b/native/cocos/bindings/jswrapper/jsvm/Object.h index 4dd684d2c47..d43bbc528f1 100644 --- a/native/cocos/bindings/jswrapper/jsvm/Object.h +++ b/native/cocos/bindings/jswrapper/jsvm/Object.h @@ -117,12 +117,6 @@ class Object : public RefCounter { BIGINT64, BIGUINT64 }; - - static std::set objBaseSet; - static bool restarting; - static void resetBaseSet() { - objBaseSet.erase(objBaseSet.begin(), objBaseSet.end()); - } using BufferContentsFreeFunc = void (*)(void *contents, size_t byteLength, void *userData); diff --git a/native/cocos/bindings/jswrapper/jsvm/ScriptEngine.cpp b/native/cocos/bindings/jswrapper/jsvm/ScriptEngine.cpp index fc65e215511..1e705c820ab 100644 --- a/native/cocos/bindings/jswrapper/jsvm/ScriptEngine.cpp +++ b/native/cocos/bindings/jswrapper/jsvm/ScriptEngine.cpp @@ -138,17 +138,14 @@ SE_BIND_FUNC(JSB_console_assert) ScriptEngine *gSriptEngineInstance = nullptr; ScriptEngine::ScriptEngine() { - static bool initialized = false; - if (initialized) { - return; - } - JSVM_InitOptions initOptions; - memset(&initOptions, 0, sizeof(initOptions)); - OH_JSVM_Init(&initOptions); - initialized = true; + OH_JSVM_Init(nullptr); + gSriptEngineInstance = this; }; -ScriptEngine::~ScriptEngine() = default; +ScriptEngine::~ScriptEngine() { + cleanup(); + gSriptEngineInstance = nullptr; +}; void ScriptEngine::setFileOperationDelegate(const FileOperationDelegate &delegate) { _fileOperationDelegate = delegate; @@ -303,6 +300,21 @@ bool ScriptEngine::init() { Object *ScriptEngine::getGlobalObject() const { return _globalObj; } +void ScriptEngine::closeEngine() { + JSVM_Env env = _env; + _env = nullptr; + + JSVM_Status status; + NODE_API_CALL(status, env, OH_JSVM_CloseEnvScope(env, _envScope)); + NODE_API_CALL(status, env, OH_JSVM_DestroyEnv(env)); + NODE_API_CALL(status, env, OH_JSVM_CloseVMScope(_vm, _vmScope)); + NODE_API_CALL(status, env, OH_JSVM_DestroyVM(_vm)); + _envScope = nullptr; + env = nullptr; + _vmScope = nullptr; + _vm = nullptr; +} + bool ScriptEngine::start() { bool ok = true; if (!init()) { @@ -337,22 +349,25 @@ void ScriptEngine::cleanup() { if (!_isValid) { return; } - Object::restarting = true; - Object::resetBaseSet(); + SE_LOGD("ScriptEngine::cleanup begin ...\n"); _isInCleanup = true; - se::AutoHandleScope hs; + + //cc::events::ScriptEngine::broadcast(cc::ScriptEngineEvent::BEFORE_CLEANUP); + do{ + se::AutoHandleScope hs; for (const auto &hook : _beforeCleanupHookArray) { hook(); } + _beforeCleanupHookArray.clear(); }while (0); - _beforeCleanupHookArray.clear(); + SAFE_DEC_REF(_globalObj); Object::cleanup(); Class::cleanup(); - garbageCollect(); + __oldConsoleLog.setUndefined(); __oldConsoleDebug.setUndefined(); @@ -360,20 +375,8 @@ void ScriptEngine::cleanup() { __oldConsoleWarn.setUndefined(); __oldConsoleError.setUndefined(); __oldConsoleAssert.setUndefined(); + garbageCollect(); - JSVM_Env env = _env; - _env = nullptr; - - JSVM_Status status; - NODE_API_CALL(status, env, OH_JSVM_CloseEnvScope(env, _envScope)); - - NODE_API_CALL(status, env, OH_JSVM_DestroyEnv(env)); - NODE_API_CALL(status, env, OH_JSVM_CloseVMScope(_vm, _vmScope)); - NODE_API_CALL(status, env, OH_JSVM_DestroyVM(_vm)); - _envScope = nullptr; - env = nullptr; - _vmScope = nullptr; - _vm = nullptr; _globalObj = nullptr; _isValid = false; diff --git a/native/cocos/bindings/jswrapper/jsvm/ScriptEngine.h b/native/cocos/bindings/jswrapper/jsvm/ScriptEngine.h index c0f7d4b17a3..e46edd4196b 100644 --- a/native/cocos/bindings/jswrapper/jsvm/ScriptEngine.h +++ b/native/cocos/bindings/jswrapper/jsvm/ScriptEngine.h @@ -77,6 +77,11 @@ class ScriptEngine { }; ScriptEngine(); ~ScriptEngine(); + /** + * @brief Shut down the JSVM engine, because cleanup doesn't destroy all objects immediately. + */ + void closeEngine(); + /** * @brief Sets the delegate for file operation. * @param delegate[in] The delegate instance for file operation. diff --git a/native/cocos/engine/Engine.cpp b/native/cocos/engine/Engine.cpp index 2497b0753a8..a5622e19e1f 100644 --- a/native/cocos/engine/Engine.cpp +++ b/native/cocos/engine/Engine.cpp @@ -207,6 +207,10 @@ void Engine::destroy() { if (cc::render::getRenderingModule()) { cc::render::Factory::destroy(cc::render::getRenderingModule()); } + #if(CC_PLATFORM == CC_PLATFORM_OPENHARMONY && SCRIPT_ENGINE_TYPE == SCRIPT_ENGINE_JSVM) + // When using JSVM, not all objects are destroyed during cleanup, so we need to close JSVM at the end. + _scriptEngine->closeEngine(); + #endif CC_SAFE_DESTROY_AND_DELETE(_gfxDevice); delete _fs; From 83ec2fec11c206700b6f204a0cd266345422dcdd Mon Sep 17 00:00:00 2001 From: qiuguohua Date: Sat, 8 Feb 2025 15:01:46 +0800 Subject: [PATCH 2/5] Remove unused code --- .../cocos/bindings/jswrapper/jsvm/ScriptEngine.cpp | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/native/cocos/bindings/jswrapper/jsvm/ScriptEngine.cpp b/native/cocos/bindings/jswrapper/jsvm/ScriptEngine.cpp index 1e705c820ab..02dfe4cf113 100644 --- a/native/cocos/bindings/jswrapper/jsvm/ScriptEngine.cpp +++ b/native/cocos/bindings/jswrapper/jsvm/ScriptEngine.cpp @@ -352,22 +352,18 @@ void ScriptEngine::cleanup() { SE_LOGD("ScriptEngine::cleanup begin ...\n"); _isInCleanup = true; - - //cc::events::ScriptEngine::broadcast(cc::ScriptEngineEvent::BEFORE_CLEANUP); - - do{ - se::AutoHandleScope hs; + se::AutoHandleScope hs; + do{ for (const auto &hook : _beforeCleanupHookArray) { hook(); } - _beforeCleanupHookArray.clear(); }while (0); - + _beforeCleanupHookArray.clear(); SAFE_DEC_REF(_globalObj); Object::cleanup(); Class::cleanup(); - + garbageCollect(); __oldConsoleLog.setUndefined(); __oldConsoleDebug.setUndefined(); @@ -375,8 +371,6 @@ void ScriptEngine::cleanup() { __oldConsoleWarn.setUndefined(); __oldConsoleError.setUndefined(); __oldConsoleAssert.setUndefined(); - garbageCollect(); - _globalObj = nullptr; _isValid = false; From db08a6d0a2ca0ab7075f53ccfbac0c4b3fb80647 Mon Sep 17 00:00:00 2001 From: qiuguohua Date: Sat, 8 Feb 2025 15:04:11 +0800 Subject: [PATCH 3/5] Adjustment of code format --- native/cocos/bindings/jswrapper/jsvm/Object.cpp | 2 -- native/cocos/bindings/jswrapper/jsvm/ScriptEngine.cpp | 3 +-- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/native/cocos/bindings/jswrapper/jsvm/Object.cpp b/native/cocos/bindings/jswrapper/jsvm/Object.cpp index efbc39f984e..1cef8628924 100644 --- a/native/cocos/bindings/jswrapper/jsvm/Object.cpp +++ b/native/cocos/bindings/jswrapper/jsvm/Object.cpp @@ -37,8 +37,6 @@ namespace se { std::unique_ptr> __objectMap; // Currently, the value `void*` is always nullptr - - Object::Object() {} Object::~Object() { if (__objectMap) { diff --git a/native/cocos/bindings/jswrapper/jsvm/ScriptEngine.cpp b/native/cocos/bindings/jswrapper/jsvm/ScriptEngine.cpp index 02dfe4cf113..1de053bcf14 100644 --- a/native/cocos/bindings/jswrapper/jsvm/ScriptEngine.cpp +++ b/native/cocos/bindings/jswrapper/jsvm/ScriptEngine.cpp @@ -349,11 +349,10 @@ void ScriptEngine::cleanup() { if (!_isValid) { return; } - SE_LOGD("ScriptEngine::cleanup begin ...\n"); _isInCleanup = true; se::AutoHandleScope hs; - do{ + do{ for (const auto &hook : _beforeCleanupHookArray) { hook(); } From 3690a5ef8d01c950f5f913e48d7013998ddd80ba Mon Sep 17 00:00:00 2001 From: qiuguohua Date: Sat, 8 Feb 2025 15:53:25 +0800 Subject: [PATCH 4/5] Fix clang-tidy errors --- native/cocos/engine/Engine.cpp | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/native/cocos/engine/Engine.cpp b/native/cocos/engine/Engine.cpp index a5622e19e1f..11f751f8277 100644 --- a/native/cocos/engine/Engine.cpp +++ b/native/cocos/engine/Engine.cpp @@ -81,9 +81,9 @@ bool setCanvasCallback(se::Object *global) { auto dpr = cc::BasePlatform::getPlatform()->getInterface()->getDevicePixelRatio(); se::Value jsbVal; - bool ok = global->getProperty("jsb", &jsbVal); + const bool ok = global->getProperty("jsb", &jsbVal); if (!jsbVal.isObject()) { - se::HandleObject jsbObj(se::Object::createPlainObject()); + const se::HandleObject jsbObj(se::Object::createPlainObject()); global->setProperty("jsb", se::Value(jsbObj)); jsbVal.setObject(jsbObj, true); } @@ -91,13 +91,13 @@ bool setCanvasCallback(se::Object *global) { se::Value windowVal; jsbVal.toObject()->getProperty("window", &windowVal); if (!windowVal.isObject()) { - se::HandleObject windowObj(se::Object::createPlainObject()); + const se::HandleObject windowObj(se::Object::createPlainObject()); jsbVal.toObject()->setProperty("window", se::Value(windowObj)); windowVal.setObject(windowObj, true); } - int width = static_cast(viewSize.width / dpr); - int height = static_cast(viewSize.height / dpr); + const int width = static_cast(viewSize.width / dpr); + const int height = static_cast(viewSize.height / dpr); windowVal.toObject()->setProperty("innerWidth", se::Value(width)); windowVal.toObject()->setProperty("innerHeight", se::Value(height)); From f7aaf52f6150f7e8367b9a3f56dd6ad7ffd4678e Mon Sep 17 00:00:00 2001 From: qiuguohua Date: Sat, 8 Feb 2025 16:05:44 +0800 Subject: [PATCH 5/5] Fix clang-tidy errors --- native/cocos/engine/Engine.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/native/cocos/engine/Engine.cpp b/native/cocos/engine/Engine.cpp index 11f751f8277..4f7a4e9b6e2 100644 --- a/native/cocos/engine/Engine.cpp +++ b/native/cocos/engine/Engine.cpp @@ -73,7 +73,7 @@ namespace { bool setCanvasCallback(se::Object *global) { - se::AutoHandleScope scope; + const se::AutoHandleScope scope; se::ScriptEngine *se = se::ScriptEngine::getInstance(); auto *window = CC_GET_MAIN_SYSTEM_WINDOW(); auto handler = window->getWindowHandle();