From 842931b5c83a13d878fb5079988f4401c758a56c Mon Sep 17 00:00:00 2001 From: 18392170496 Date: Mon, 31 Jul 2023 21:50:05 +0800 Subject: [PATCH] fix mem leak Signed-off-by: 18392170496 --- .../include/napi_remote_object_holder.h | 15 +- .../include/napi_remote_object_internal.h | 46 ++++-- .../napi_common/source/napi_remote_object.cpp | 154 +++++++++++++++--- .../source/napi_remote_object_holder.cpp | 67 +++++--- 4 files changed, 214 insertions(+), 68 deletions(-) diff --git a/ipc/native/src/napi_common/include/napi_remote_object_holder.h b/ipc/native/src/napi_common/include/napi_remote_object_holder.h index 2d1ca91..eb53fba 100644 --- a/ipc/native/src/napi_common/include/napi_remote_object_holder.h +++ b/ipc/native/src/napi_common/include/napi_remote_object_holder.h @@ -26,12 +26,14 @@ namespace OHOS { class NAPIRemoteObjectHolder : public RefBase { public: - explicit NAPIRemoteObjectHolder(napi_env env, const std::u16string &descriptor); - ~NAPIRemoteObjectHolder(); - sptr Get(napi_value object); - void Set(sptr object); + explicit NAPIRemoteObjectHolder(napi_env env, const std::u16string &descriptor, napi_value thisVar); + ~NAPIRemoteObjectHolder() = default; + sptr Get(); + void Set(sptr object); void attachLocalInterface(napi_value localInterface, std::string &descriptor); napi_value queryLocalInterface(std::string &descriptor); + napi_ref GetJsObjectRef() const; + napi_env GetJsObjectEnv() const; void CleanJsEnv(); void Lock() { @@ -64,10 +66,13 @@ public: private: std::mutex mutex_; napi_env env_ = nullptr; + std::thread::id jsThreadId_; std::u16string descriptor_; - sptr cachedObject_; + sptr sptrCachedObject_; + wptr wptrCachedObject_; napi_ref localInterfaceRef_; int32_t attachCount_; + napi_ref jsObjectRef_; }; } // namespace OHOS #endif // NAPI_REMOTE_OBJECT_HOLDER_H \ No newline at end of file diff --git a/ipc/native/src/napi_common/include/napi_remote_object_internal.h b/ipc/native/src/napi_common/include/napi_remote_object_internal.h index 7f1c51c..de9acf6 100644 --- a/ipc/native/src/napi_common/include/napi_remote_object_internal.h +++ b/ipc/native/src/napi_common/include/napi_remote_object_internal.h @@ -16,6 +16,8 @@ #ifndef NAPI_IPC_OHOS_REMOTE_OBJECT_INTERNAL_H #define NAPI_IPC_OHOS_REMOTE_OBJECT_INTERNAL_H +#include + #include "ipc_object_stub.h" #include "iremote_object.h" #include "message_parcel.h" @@ -24,9 +26,33 @@ #include "napi_remote_object.h" namespace OHOS { +struct ThreadLockInfo { + std::mutex mutex; + std::condition_variable condition; + bool ready = false; +}; + +struct CallbackParam { + napi_env env; + napi_ref thisVarRef; + uint32_t code; + MessageParcel *data; + MessageParcel *reply; + MessageOption *option; + CallingInfo callingInfo; + ThreadLockInfo *lockInfo; + int result; +}; + +struct OperateJsRefParam { + napi_env env; + napi_ref thisVarRef; + ThreadLockInfo *lockInfo; +}; + class NAPIRemoteObject : public IPCObjectStub { public: - NAPIRemoteObject(napi_env env, napi_value thisVar, const std::u16string &descriptor); + NAPIRemoteObject(std::thread::id jsThreadId, napi_env env, napi_ref jsObjectRef, const std::u16string &descriptor); ~NAPIRemoteObject() override; @@ -41,26 +67,10 @@ public: void ResetJsEnv(); private: napi_env env_ = nullptr; - napi_value thisVar_ = nullptr; + std::thread::id jsThreadId_; static napi_value ThenCallback(napi_env env, napi_callback_info info); static napi_value CatchCallback(napi_env env, napi_callback_info info); napi_ref thisVarRef_ = nullptr; - struct ThreadLockInfo { - std::mutex mutex; - std::condition_variable condition; - bool ready = false; - }; - struct CallbackParam { - napi_env env; - napi_ref thisVarRef; - uint32_t code; - MessageParcel *data; - MessageParcel *reply; - MessageOption *option; - CallingInfo callingInfo; - ThreadLockInfo *lockInfo; - int result; - }; int OnJsRemoteRequest(CallbackParam *jsParam); }; diff --git a/ipc/native/src/napi_common/source/napi_remote_object.cpp b/ipc/native/src/napi_common/source/napi_remote_object.cpp index fc7d28c..db34409 100644 --- a/ipc/native/src/napi_common/source/napi_remote_object.cpp +++ b/ipc/native/src/napi_common/source/napi_remote_object.cpp @@ -17,7 +17,6 @@ #include #include -#include #include #include "iremote_invoker.h" @@ -60,15 +59,84 @@ static void RemoteObjectHolderFinalizeCb(napi_env env, void *data, void *hint) } holder->Lock(); int32_t curAttachCount = holder->DecAttachCount(); + ZLOGD(LOG_LABEL, "NAPIRemoteObjectHolder destructed by js callback, curAttachCount:%{public}d", curAttachCount); if (curAttachCount == 0) { delete holder; } } +static void DecreaseJsObjectRef(napi_env env, napi_ref ref) +{ + if (ref == nullptr) { + ZLOGI(LOG_LABEL, "ref is nullptr, do nothing"); + return; + } + + uint32_t result; + napi_status napiStatus = napi_reference_unref(env, ref, &result); + NAPI_ASSERT_RETURN_VOID(env, napiStatus == napi_ok, "failed to decrease ref to js RemoteObject"); +} + +static void IncreaseJsObjectRef(napi_env env, napi_ref ref) +{ + uint32_t result; + napi_status napiStatus = napi_reference_ref(env, ref, &result); + NAPI_ASSERT_RETURN_VOID(env, napiStatus == napi_ok, "failed to increase ref to js RemoteObject"); +} + +static void RemoteObjectHolderRefCb(napi_env env, void *data, void *hint) +{ + NAPIRemoteObjectHolder *holder = reinterpret_cast(data); + if (holder == nullptr) { + ZLOGW(LOG_LABEL, "RemoteObjectHolderRefCb holder is nullptr"); + return; + } + holder->Lock(); + int32_t curAttachCount = holder->DecAttachCount(); + holder->Unlock(); + ZLOGD(LOG_LABEL, "RemoteObjectHolderRefCb, curAttachCount:%{public}d", curAttachCount); + + napi_ref ref = holder->GetJsObjectRef(); + napi_env workerEnv = holder->GetJsObjectEnv(); + if (ref == nullptr || workerEnv == nullptr) { + ZLOGE(LOG_LABEL, "ref or env is null"); + return; + } + uv_loop_s *loop = nullptr; + napi_get_uv_event_loop(workerEnv, &loop); + uv_work_t *work = new(std::nothrow) uv_work_t; + NAPI_ASSERT_RETURN_VOID(workerEnv, work != nullptr, "cb failed to new work"); + OperateJsRefParam *param = new OperateJsRefParam { + .env = workerEnv, + .thisVarRef = ref + }; + work->data = reinterpret_cast(param); + uv_queue_work(loop, work, [](uv_work_t *work) {}, [](uv_work_t *work, int status) { + ZLOGI(LOG_LABEL, "decrease on uv work thread"); + OperateJsRefParam *param = reinterpret_cast(work->data); + napi_handle_scope scope = nullptr; + napi_open_handle_scope(param->env, &scope); + DecreaseJsObjectRef(param->env, param->thisVarRef); + napi_close_handle_scope(param->env, scope); + delete param; + delete work; + }); +} + static void *RemoteObjectDetachCb(NativeEngine *engine, void *value, void *hint) { - (void)engine; (void)hint; + napi_env env = reinterpret_cast(engine); + NAPIRemoteObjectHolder *holder = reinterpret_cast(value); + napi_ref ref = holder->GetJsObjectRef(); + + uint32_t result; + napi_status napiStatus = napi_reference_ref(env, ref, &result); + if (napiStatus != napi_ok) { + ZLOGE(LOG_LABEL, "RemoteObjectDetachCb, failed to increase ref"); + } else { + ZLOGI(LOG_LABEL, "RemoteObjectDetachCb, ref result:%{public}u", result); + } return value; } @@ -106,7 +174,7 @@ static NativeValue *RemoteObjectAttachCb(NativeEngine *engine, void *value, void NAPIRemoteObjectHolder *createHolder = nullptr; status = napi_remove_wrap(env, jsRemoteObject, (void **)&createHolder); NAPI_ASSERT(env, status == napi_ok && createHolder != nullptr, "failed to remove create holder when attach"); - status = napi_wrap(env, jsRemoteObject, holder, RemoteObjectHolderFinalizeCb, nullptr, nullptr); + status = napi_wrap(env, jsRemoteObject, holder, RemoteObjectHolderRefCb, nullptr, nullptr); NAPI_ASSERT(env, status == napi_ok, "wrap js RemoteObject and native holder failed when attach"); holder->IncAttachCount(); holder->Unlock(); @@ -145,7 +213,7 @@ napi_value RemoteObject_JS_Constructor(napi_env env, napi_callback_info info) napi_get_value_string_utf8(env, argv[0], stringValue, bufferSize + 1, &jsStringLength); NAPI_ASSERT(env, jsStringLength == bufferSize, "string length wrong"); std::string descriptor = stringValue; - auto holder = new NAPIRemoteObjectHolder(env, Str8ToStr16(descriptor)); + auto holder = new NAPIRemoteObjectHolder(env, Str8ToStr16(descriptor), thisVar); auto nativeObj = ConvertNativeValueTo(reinterpret_cast(thisVar)); if (nativeObj == nullptr) { ZLOGE(LOG_LABEL, "Failed to get RemoteObject native object"); @@ -156,31 +224,78 @@ napi_value RemoteObject_JS_Constructor(napi_env env, napi_callback_info info) // connect native object to js thisVar napi_status status = napi_wrap(env, thisVar, holder, RemoteObjectHolderFinalizeCb, nullptr, nullptr); NAPI_ASSERT(env, status == napi_ok, "wrap js RemoteObject and native holder failed"); - if (NAPI_ohos_rpc_getNativeRemoteObject(env, thisVar) == nullptr) { - ZLOGE(LOG_LABEL, "RemoteObject_JS_Constructor create native object failed"); - return nullptr; - } // register listener for env destruction status = napi_add_env_cleanup_hook(env, OnEnvCleanUp, holder); NAPI_ASSERT(env, status == napi_ok, "add cleanup hook failed"); return thisVar; } -NAPIRemoteObject::NAPIRemoteObject(napi_env env, napi_value thisVar, const std::u16string &descriptor) +NAPIRemoteObject::NAPIRemoteObject(std::thread::id jsThreadId, napi_env env, napi_ref jsObjectRef, + const std::u16string &descriptor) : IPCObjectStub(descriptor) { env_ = env; - thisVar_ = thisVar; - napi_create_reference(env, thisVar_, 1, &thisVarRef_); - NAPI_ASSERT_RETURN_VOID(env, thisVarRef_ != nullptr, "failed to create ref to js RemoteObject"); + jsThreadId_ = jsThreadId; + thisVarRef_ = jsObjectRef; + + if (jsThreadId_ == std::this_thread::get_id()) { + IncreaseJsObjectRef(env, jsObjectRef); + } else { + uv_loop_s *loop = nullptr; + napi_get_uv_event_loop(env_, &loop); + uv_work_t *work = new(std::nothrow) uv_work_t; + NAPI_ASSERT_RETURN_VOID(env_, work != nullptr, "create NAPIRemoteObject, new work failed"); + std::shared_ptr lockInfo = std::make_shared(); + OperateJsRefParam *param = new OperateJsRefParam { + .env = env_, + .thisVarRef = jsObjectRef, + .lockInfo = lockInfo.get() + }; + + work->data = reinterpret_cast(param); + uv_queue_work(loop, work, [](uv_work_t *work) {}, [](uv_work_t *work, int status) { + OperateJsRefParam *param = reinterpret_cast(work->data); + napi_handle_scope scope = nullptr; + napi_open_handle_scope(param->env, &scope); + IncreaseJsObjectRef(param->env, param->thisVarRef); + std::unique_lock lock(param->lockInfo->mutex); + param->lockInfo->ready = true; + param->lockInfo->condition.notify_all(); + napi_close_handle_scope(param->env, scope); + }); + std::unique_lock lock(param->lockInfo->mutex); + param->lockInfo->condition.wait(lock, [¶m] { return param->lockInfo->ready; }); + delete param; + delete work; + } } NAPIRemoteObject::~NAPIRemoteObject() { ZLOGI(LOG_LABEL, "NAPIRemoteObject Destructor"); if (thisVarRef_ != nullptr && env_ != nullptr) { - napi_status status = napi_delete_reference(env_, thisVarRef_); - NAPI_ASSERT_RETURN_VOID(env_, status == napi_ok, "failed to delete ref to js RemoteObject"); + if (jsThreadId_ == std::this_thread::get_id()) { + DecreaseJsObjectRef(env_, thisVarRef_); + } else { + uv_loop_s *loop = nullptr; + napi_get_uv_event_loop(env_, &loop); + uv_work_t *work = new(std::nothrow) uv_work_t; + NAPI_ASSERT_RETURN_VOID(env_, work != nullptr, "release NAPIRemoteObject, new work failed"); + OperateJsRefParam *param = new OperateJsRefParam { + .env = env_, + .thisVarRef = thisVarRef_ + }; + work->data = reinterpret_cast(param); + uv_queue_work(loop, work, [](uv_work_t *work) {}, [](uv_work_t *work, int status) { + OperateJsRefParam *param = reinterpret_cast(work->data); + napi_handle_scope scope = nullptr; + napi_open_handle_scope(param->env, &scope); + DecreaseJsObjectRef(param->env, param->thisVarRef); + napi_close_handle_scope(param->env, scope); + delete param; + delete work; + }); + } thisVarRef_ = nullptr; } } @@ -604,12 +719,11 @@ napi_value NAPI_ohos_rpc_CreateJsRemoteObject(napi_env env, const sptrCheckObjectLegality()) { + if (!target->IsProxyObject()) { IPCObjectStub *tmp = static_cast(target.GetRefPtr()); uint32_t objectType = tmp->GetObjectType(); - ZLOGI(LOG_LABEL, "object type:%{public}d", objectType); + ZLOGI(LOG_LABEL, "create js object, type:%{public}d", objectType); if (objectType == IPCObjectStub::OBJECT_TYPE_JAVASCRIPT || objectType == IPCObjectStub::OBJECT_TYPE_NATIVE) { - sptr object = static_cast(target.GetRefPtr()); // retrieve js remote object constructor napi_value global = nullptr; napi_status status = napi_get_global(env, &global); @@ -619,7 +733,7 @@ napi_value NAPI_ohos_rpc_CreateJsRemoteObject(napi_env env, const sptrGetObjectDescriptor(); + std::u16string descriptor = target->GetObjectDescriptor(); std::string desc = Str16ToStr8(descriptor); napi_value jsDesc = nullptr; napi_create_string_utf8(env, desc.c_str(), desc.length(), &jsDesc); @@ -633,7 +747,7 @@ napi_value NAPI_ohos_rpc_CreateJsRemoteObject(napi_env env, const sptrSet(object); + holder->Set(target); return jsRemoteObject; } } @@ -670,7 +784,7 @@ sptr NAPI_ohos_rpc_getNativeRemoteObject(napi_env env, napi_value NAPIRemoteObjectHolder *holder = nullptr; napi_unwrap(env, object, (void **)&holder); NAPI_ASSERT(env, holder != nullptr, "failed to get napi remote object holder"); - return holder != nullptr ? holder->Get(object) : nullptr; + return holder != nullptr ? holder->Get() : nullptr; } napi_value proxyConstructor = nullptr; diff --git a/ipc/native/src/napi_common/source/napi_remote_object_holder.cpp b/ipc/native/src/napi_common/source/napi_remote_object_holder.cpp index eed9cf3..56a479b 100644 --- a/ipc/native/src/napi_common/source/napi_remote_object_holder.cpp +++ b/ipc/native/src/napi_common/source/napi_remote_object_holder.cpp @@ -15,6 +15,7 @@ #include "napi_remote_object_holder.h" +#include #include #include "ipc_debug.h" #include "log_tags.h" @@ -22,47 +23,66 @@ namespace OHOS { static constexpr OHOS::HiviewDFX::HiLogLabel LOG_LABEL = { LOG_CORE, LOG_ID_IPC, "napi_remoteObject_holder" }; -NAPIRemoteObjectHolder::NAPIRemoteObjectHolder(napi_env env, const std::u16string &descriptor) - : env_(env), descriptor_(descriptor), cachedObject_(nullptr), localInterfaceRef_(nullptr), attachCount_(1) -{} +struct DeleteJsRefParam { + napi_env env; + napi_ref thisVarRef; +}; -NAPIRemoteObjectHolder::~NAPIRemoteObjectHolder() +NAPIRemoteObjectHolder::NAPIRemoteObjectHolder(napi_env env, const std::u16string &descriptor, napi_value thisVar) + : env_(env), descriptor_(descriptor), sptrCachedObject_(nullptr), wptrCachedObject_(nullptr), + localInterfaceRef_(nullptr), attachCount_(1), jsObjectRef_(nullptr) { - // free the reference of object. - cachedObject_ = nullptr; - if (localInterfaceRef_ != nullptr) { - napi_delete_reference(env_, localInterfaceRef_); - } + jsThreadId_ = std::this_thread::get_id(); + // create weak ref, do not need to delete, + // increase ref count when the JS object will transfer to another thread or process. + napi_create_reference(env, thisVar, 0, &jsObjectRef_); } -sptr NAPIRemoteObjectHolder::Get(napi_value jsRemoteObject) +sptr NAPIRemoteObjectHolder::Get() { std::lock_guard lockGuard(mutex_); // grab an strong reference to the object, // so it will not be freed util this reference released. - sptr remoteObject = nullptr; - if (cachedObject_ != nullptr) { - remoteObject = cachedObject_; + if (sptrCachedObject_ != nullptr) { + return sptrCachedObject_; } - if (remoteObject == nullptr) { - remoteObject = new NAPIRemoteObject(env_, jsRemoteObject, descriptor_); - cachedObject_ = remoteObject; + sptr tmp = wptrCachedObject_.promote(); + if (tmp == nullptr && env_ != nullptr) { + tmp = new NAPIRemoteObject(jsThreadId_, env_, jsObjectRef_, descriptor_); + wptrCachedObject_ = tmp; } - return remoteObject; + return tmp; } -void NAPIRemoteObjectHolder::Set(sptr object) +void NAPIRemoteObjectHolder::Set(sptr object) { std::lock_guard lockGuard(mutex_); - cachedObject_ = object; + IPCObjectStub *tmp = static_cast(object.GetRefPtr()); + if (tmp->GetObjectType() == IPCObjectStub::OBJECT_TYPE_JAVASCRIPT) { + wptrCachedObject_ = object; + } else { + sptrCachedObject_ = object; + } +} + +napi_ref NAPIRemoteObjectHolder::GetJsObjectRef() const +{ + return jsObjectRef_; +} + +napi_env NAPIRemoteObjectHolder::GetJsObjectEnv() const +{ + return env_; } void NAPIRemoteObjectHolder::CleanJsEnv() { env_ = nullptr; - sptr object = cachedObject_; - if (object != nullptr) { + jsObjectRef_ = nullptr; + sptr tmp = wptrCachedObject_.promote(); + if (tmp != nullptr) { + NAPIRemoteObject *object = static_cast(tmp.GetRefPtr()); ZLOGI(LOG_LABEL, "reset env and napi_ref"); object->ResetJsEnv(); } @@ -74,10 +94,7 @@ void NAPIRemoteObjectHolder::attachLocalInterface(napi_value localInterface, std ZLOGE(LOG_LABEL, "Js env has been destructed"); return; } - if (localInterfaceRef_ != nullptr) { - napi_delete_reference(env_, localInterfaceRef_); - } - napi_create_reference(env_, localInterface, 1, &localInterfaceRef_); + napi_create_reference(env_, localInterface, 0, &localInterfaceRef_); descriptor_ = Str8ToStr16(descriptor); } -- GitLab