!736 回退NAPIRemoteObject内存泄漏相关修改

Merge pull request !736 from 马根堂/master

ipc/native/src/core/source/ipc_object_stub.cpp

@@ -371,7 +371,7 @@ int IPCObjectStub::GetObjectType() const
 int32_t IPCObjectStub::ProcessProto(uint32_t code, MessageParcel &data, MessageParcel &reply, MessageOption &option)
 {
     int result = ERR_NONE;
-    ZLOGD(LABEL, "normal stub object, descriptor_=%{public}s", Str16ToStr8(descriptor_).c_str());
+    ZLOGD(LABEL, "IPCObjectStub::ProcessProto called, type = 0, normal stub object");
     if (!reply.WriteUint32(IRemoteObject::IF_PROT_BINDER)) {
         ZLOGE(LABEL, "write to parcel fail");
         result = IPC_STUB_WRITE_PARCEL_ERR;

ipc/native/src/napi_common/include/napi_remote_object_holder.h

@@ -26,9 +26,9 @@
 namespace OHOS {
 class NAPIRemoteObjectHolder : public RefBase {
 public:
-    explicit NAPIRemoteObjectHolder(napi_env env, const std::u16string &descriptor, napi_value thisVar);
+    explicit NAPIRemoteObjectHolder(napi_env env, const std::u16string &descriptor);
     ~NAPIRemoteObjectHolder();
-    sptr<NAPIRemoteObject> Get(napi_env envNew);
+    sptr<NAPIRemoteObject> Get(napi_value object);
     void Set(sptr<NAPIRemoteObject> object);
     void attachLocalInterface(napi_value localInterface, std::string &descriptor);
     napi_value queryLocalInterface(std::string &descriptor);
@@ -64,11 +64,9 @@ private:
     std::mutex mutex_;
     napi_env env_ = nullptr;
     std::u16string descriptor_;
-    sptr<NAPIRemoteObject> sptrCachedObject_;
-    wptr<NAPIRemoteObject> wptrCachedObject_;
+    sptr<NAPIRemoteObject> cachedObject_;
     napi_ref localInterfaceRef_;
     int32_t attachCount_;
-    napi_ref jsObjectRef_ = nullptr;
 };
 } // namespace OHOS
 #endif // NAPI_REMOTE_OBJECT_HOLDER_H
\ No newline at end of file

ipc/native/src/napi_common/include/napi_remote_object_internal.h

@@ -26,7 +26,7 @@
 namespace OHOS {
 class NAPIRemoteObject : public IPCObjectStub {
 public:
-    NAPIRemoteObject(napi_env envNew, napi_env env, napi_ref jsObjectRef, const std::u16string &descriptor);
+    NAPIRemoteObject(napi_env env, napi_value thisVar, const std::u16string &descriptor);
 
     ~NAPIRemoteObject() override;
 
@@ -37,12 +37,8 @@ public:
     int OnRemoteRequest(uint32_t code, MessageParcel &data, MessageParcel &reply, MessageOption &option) override;
 
     napi_ref GetJsObjectRef() const;
-
-    void SetNewEnv(napi_env envNew);
 private:
-    std::u16string desc_;
     napi_env env_ = nullptr;
-    napi_env envNew_ = nullptr;
     napi_value thisVar_ = nullptr;
     static napi_value ThenCallback(napi_env env, napi_callback_info info);
     static napi_value CatchCallback(napi_env env, napi_callback_info info);

ipc/native/src/napi_common/source/napi_remote_object.cpp

@@ -60,7 +60,6 @@ static void RemoteObjectHolderFinalizeCb(napi_env env, void *data, void *hint)
     }
     holder->Lock();
     int32_t curAttachCount = holder->DecAttachCount();
-    ZLOGI(LOG_LABEL, "NAPIRemoteObjectHolder destructed by js callback, curAttachCount:%{public}d", curAttachCount);
     if (curAttachCount == 0) {
         delete holder;
     }
@@ -135,7 +134,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), thisVar);
+    auto holder = new NAPIRemoteObjectHolder(env, Str8ToStr16(descriptor));
     auto nativeObj = ConvertNativeValueTo<NativeObject>(reinterpret_cast<NativeValue *>(thisVar));
     if (nativeObj == nullptr) {
         ZLOGE(LOG_LABEL, "Failed to get RemoteObject native object");
@@ -146,19 +145,17 @@ 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;
+    }
     return thisVar;
 }
 
-NAPIRemoteObject::NAPIRemoteObject(napi_env envNew, napi_env env, napi_ref jsObjectRef,
-    const std::u16string &descriptor)
+NAPIRemoteObject::NAPIRemoteObject(napi_env env, napi_value thisVar, const std::u16string &descriptor)
     : IPCObjectStub(descriptor)
 {
-    envNew_ = envNew;
     env_ = env;
-    desc_ = descriptor;
-    napi_value thisVar = nullptr;
-    napi_get_reference_value(env, jsObjectRef, &thisVar);
-    NAPI_ASSERT_RETURN_VOID(env, thisVar != nullptr, "failed to get value from js RemoteObject ref");
     thisVar_ = thisVar;
     napi_create_reference(env, thisVar_, 1, &thisVarRef_);
     NAPI_ASSERT_RETURN_VOID(env, thisVarRef_ != nullptr, "failed to create ref to js RemoteObject");
@@ -166,8 +163,8 @@ NAPIRemoteObject::NAPIRemoteObject(napi_env envNew, napi_env env, napi_ref jsObj
 
 NAPIRemoteObject::~NAPIRemoteObject()
 {
-    ZLOGI(LOG_LABEL, "NAPIRemoteObject destroyed, desc:%{public}s", Str16ToStr8(desc_).c_str());
-    if (thisVarRef_ != nullptr && envNew_ == env_) {
+    ZLOGI(LOG_LABEL, "NAPIRemoteObject Destructor");
+    if (thisVarRef_ != nullptr) {
         napi_status status = napi_delete_reference(env_, thisVarRef_);
         NAPI_ASSERT_RETURN_VOID(env_, status == napi_ok, "failed to delete ref to js RemoteObject");
         thisVarRef_ = nullptr;
@@ -189,14 +186,6 @@ napi_ref NAPIRemoteObject::GetJsObjectRef() const
     return thisVarRef_;
 }
 
-void NAPIRemoteObject::SetNewEnv(napi_env envNew)
-{
-    if (envNew_ != envNew) {
-        ZLOGI(LOG_LABEL, "env updated");
-    }
-    envNew_ = envNew;
-}
-
 void NAPI_RemoteObject_getCallingInfo(CallingInfo &newCallingInfoParam)
 {
     newCallingInfoParam.callingPid = IPCSkeleton::GetCallingPid();
@@ -670,7 +659,7 @@ sptr<IRemoteObject> 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(env) : nullptr;
+            return holder != nullptr ? holder->Get(object) : nullptr;
         }
 
         napi_value proxyConstructor = nullptr;

ipc/native/src/napi_common/source/napi_remote_object_holder.cpp

@@ -18,56 +18,40 @@
 #include <string_ex.h>
 
 namespace OHOS {
-NAPIRemoteObjectHolder::NAPIRemoteObjectHolder(napi_env env, const std::u16string &descriptor, napi_value thisVar)
-{
-    env_ = env;
-    descriptor_ = descriptor;
-    sptrCachedObject_ = nullptr;
-    wptrCachedObject_ = nullptr;
-    localInterfaceRef_ = nullptr;
-    attachCount_ = 1;
-    napi_create_reference(env, thisVar, 0, &jsObjectRef_);
-}
+NAPIRemoteObjectHolder::NAPIRemoteObjectHolder(napi_env env, const std::u16string &descriptor)
+    : env_(env), descriptor_(descriptor), cachedObject_(nullptr), localInterfaceRef_(nullptr), attachCount_(1)
+{}
 
 NAPIRemoteObjectHolder::~NAPIRemoteObjectHolder()
 {
     // free the reference of object.
-    sptrCachedObject_ = nullptr;
-    wptrCachedObject_ = nullptr;
+    cachedObject_ = nullptr;
     if (localInterfaceRef_ != nullptr) {
         napi_delete_reference(env_, localInterfaceRef_);
     }
-    if (jsObjectRef_ != nullptr) {
-        napi_delete_reference(env_, jsObjectRef_);
-    }
 }
 
-sptr<NAPIRemoteObject> NAPIRemoteObjectHolder::Get(napi_env envNew)
+sptr<NAPIRemoteObject> NAPIRemoteObjectHolder::Get(napi_value jsRemoteObject)
 {
     std::lock_guard<std::mutex> lockGuard(mutex_);
     // grab an strong reference to the object,
     // so it will not be freed util this reference released.
-    if (sptrCachedObject_ != nullptr) {
-        return sptrCachedObject_;
+    sptr<NAPIRemoteObject> remoteObject = nullptr;
+    if (cachedObject_ != nullptr) {
+        remoteObject = cachedObject_;
     }
-    sptr<NAPIRemoteObject> tmp = wptrCachedObject_.promote();
-    if (tmp == nullptr) {
-        tmp = new NAPIRemoteObject(envNew, env_, jsObjectRef_, descriptor_);
-        wptrCachedObject_ = tmp;
+
+    if (remoteObject == nullptr) {
+        remoteObject = new NAPIRemoteObject(env_, jsRemoteObject, descriptor_);
+        cachedObject_ = remoteObject;
     }
-    tmp->SetNewEnv(envNew);
-    return tmp;
+    return remoteObject;
 }
 
 void NAPIRemoteObjectHolder::Set(sptr<NAPIRemoteObject> object)
 {
     std::lock_guard<std::mutex> lockGuard(mutex_);
-    IPCObjectStub *tmp = static_cast<IPCObjectStub *>(object.GetRefPtr());
-    if (tmp->GetObjectType() == IPCObjectStub::OBJECT_TYPE_JAVASCRIPT) {
-        wptrCachedObject_ = object;
-    } else {
-        sptrCachedObject_ = object;
-    }
+    cachedObject_ = object;
 }
 
 void NAPIRemoteObjectHolder::attachLocalInterface(napi_value localInterface, std::string &descriptor)