From af5fbc4a863f47a8975ccd0f33858795aad18cfd Mon Sep 17 00:00:00 2001 From: ilikethese Date: Tue, 17 Oct 2023 13:14:34 +0800 Subject: [PATCH] fix(driver): fix multi thead conflict when destroy engine --- driver/js/include/driver/js_driver_utils.h | 4 +- driver/js/src/js_driver_utils.cc | 49 +++++++++++-------- .../js/src/main/cpp/src/js_driver_jni.cc | 5 +- 3 files changed, 34 insertions(+), 24 deletions(-) diff --git a/driver/js/include/driver/js_driver_utils.h b/driver/js/include/driver/js_driver_utils.h index dca8fb8e452..7a9a36cad04 100644 --- a/driver/js/include/driver/js_driver_utils.h +++ b/driver/js/include/driver/js_driver_utils.h @@ -53,8 +53,8 @@ class JsDriverUtils { const string_view& global_config, std::function)>&& scope_initialized_callback, const JsCallback& call_host_callback); - static void DestroyInstance(const std::shared_ptr& engine, - const std::shared_ptr& scope, + static void DestroyInstance(std::shared_ptr&& engine, + std::shared_ptr&& scope, const std::function& callback, bool is_reload); static bool RunScript(const std::shared_ptr& scope, diff --git a/driver/js/src/js_driver_utils.cc b/driver/js/src/js_driver_utils.cc index 0ecce2155f3..1e9bfc4af4a 100644 --- a/driver/js/src/js_driver_utils.cc +++ b/driver/js/src/js_driver_utils.cc @@ -379,28 +379,10 @@ bool JsDriverUtils::RunScript(const std::shared_ptr& scope, return flag; } -void JsDriverUtils::DestroyInstance(const std::shared_ptr& engine, - const std::shared_ptr& scope, +void JsDriverUtils::DestroyInstance(std::shared_ptr&& engine, + std::shared_ptr&& scope, const std::function& callback, bool is_reload) { - auto scope_destroy_callback = [engine, scope, is_reload, callback] { -#if defined(JS_V8) && defined(ENABLE_INSPECTOR) && !defined(V8_WITHOUT_INSPECTOR) - auto v8_vm = std::static_pointer_cast(engine->GetVM()); - if (v8_vm->IsDebug()) { - auto inspector_client = v8_vm->GetInspectorClient(); - if (inspector_client) { - auto inspector_context = scope->GetInspectorContext(); - inspector_client->DestroyInspectorContext(is_reload, inspector_context); - } - } else { - scope->WillExit(); - } -#else - scope->WillExit(); -#endif - FOOTSTONE_LOG(INFO) << "js destroy end"; - callback(true); - }; auto vm = engine->GetVM(); auto group = vm->GetGroupId(); if (vm->IsDebug()) { @@ -423,6 +405,33 @@ void JsDriverUtils::DestroyInstance(const std::shared_ptr& engine, } } auto runner = engine->GetJsTaskRunner(); + + // DestroyInstance is called in the bridge thread, while the + // scope_destroy_callback is pushed into the task_runner to run in the DOM + // thread. When DestroyInstance is called, it holds the scope and engine. + // However, executing scope_destroy_callback may add tasks (e.g., setTimeout) + // holding the scope, even when the engine is destroyed. To prevent this, move + // the scope and engine into the scope_destroy_callback, which will be released + // when the callback is executed, ensuring no other tasks can be added to the + // task runner. + auto scope_destroy_callback = [engine = std::move(engine), scope = std::move(scope), is_reload, callback] { +#if defined(JS_V8) && defined(ENABLE_INSPECTOR) && !defined(V8_WITHOUT_INSPECTOR) + auto v8_vm = std::static_pointer_cast(engine->GetVM()); + if (v8_vm->IsDebug()) { + auto inspector_client = v8_vm->GetInspectorClient(); + if (inspector_client) { + auto inspector_context = scope->GetInspectorContext(); + inspector_client->DestroyInspectorContext(is_reload, inspector_context); + } + } else { + scope->WillExit(); + } +#else + scope->WillExit(); +#endif + FOOTSTONE_LOG(INFO) << "js destroy end"; + callback(true); + }; runner->PostTask(std::move(scope_destroy_callback)); FOOTSTONE_DLOG(INFO) << "destroy, group = " << group; } diff --git a/framework/android/connector/driver/js/src/main/cpp/src/js_driver_jni.cc b/framework/android/connector/driver/js/src/main/cpp/src/js_driver_jni.cc index b2eff77f78b..9f56462191a 100644 --- a/framework/android/connector/driver/js/src/main/cpp/src/js_driver_jni.cc +++ b/framework/android/connector/driver/js/src/main/cpp/src/js_driver_jni.cc @@ -51,6 +51,8 @@ #include "vfs/uri.h" #include "vfs/vfs_resource_holder.h" +#include + #ifdef JS_V8 #include "driver/vm/v8/v8_vm.h" #endif @@ -368,14 +370,13 @@ void DestroyJsDriver(__unused JNIEnv* j_env, auto scope_id = footstone::checked_numeric_cast(j_scope_id); auto flag = hippy::global_data_holder.Erase(scope_id); FOOTSTONE_CHECK(flag); - JsDriverUtils::DestroyInstance(engine, scope, [bridge_callback_object](bool ret) { + JsDriverUtils::DestroyInstance(std::move(engine), std::move(scope), [bridge_callback_object](bool ret) { if (ret) { hippy::bridge::CallJavaMethod(bridge_callback_object->GetObj(),INIT_CB_STATE::SUCCESS); } else { hippy::bridge::CallJavaMethod(bridge_callback_object->GetObj(),INIT_CB_STATE::DESTROY_ERROR); } }, static_cast(j_is_reload)); - scope = nullptr; } void LoadInstance(JNIEnv* j_env,