fix(dom): fix dom memory leak (#3628)

Co-authored-by: OpenHippy <[email protected]>
Tencent · Nov 22, 2023 · 117e087 · 117e087
1 parent be23474
commit 117e087
Show file tree

Hide file tree

Showing 6 changed files with 56 additions and 11 deletions.
diff --git a/dom/src/dom/deserializer_unittests.cc b/dom/src/dom/deserializer_unittests.cc
@@ -44,6 +44,7 @@ void CheckUint32(uint32_t value) {
 
   footstone::value::HippyValue hippy_value;
   deserializer.ReadObject(hippy_value);
+  footstone::value::SerializerHelper::DestroyBuffer(buffer);
   EXPECT_TRUE(hippy_value.GetType() == footstone::value::HippyValue::Type::kNumber);
   EXPECT_TRUE(hippy_value.GetNumberType() == footstone::value::HippyValue::NumberType::kUInt32);
   EXPECT_TRUE(hippy_value.ToUint32Checked() == value);
@@ -60,6 +61,7 @@ void CheckInt32(int32_t value) {
 
   footstone::value::HippyValue hippy_value;
   deserializer.ReadObject(hippy_value);
+  footstone::value::SerializerHelper::DestroyBuffer(buffer);
   EXPECT_TRUE(hippy_value.GetType() == footstone::value::HippyValue::Type::kNumber);
   EXPECT_TRUE(hippy_value.GetNumberType() == footstone::value::HippyValue::NumberType::kInt32);
   EXPECT_TRUE(hippy_value.ToInt32Checked() == value);
@@ -76,6 +78,7 @@ void CheckDouble(double value) {
 
   footstone::value::HippyValue hippy_value;
   deserializer.ReadObject(hippy_value);
+  footstone::value::SerializerHelper::DestroyBuffer(buffer);
   EXPECT_TRUE(hippy_value.GetType() == footstone::value::HippyValue::Type::kNumber);
   EXPECT_TRUE(hippy_value.GetNumberType() == footstone::value::HippyValue::NumberType::kDouble);
   EXPECT_TRUE(hippy_value.ToDoubleChecked() == value);
@@ -92,6 +95,7 @@ void CheckString(std::string value) {
 
   footstone::value::HippyValue hippy_value;
   deserializer.ReadObject(hippy_value);
+  footstone::value::SerializerHelper::DestroyBuffer(buffer);
   EXPECT_TRUE(hippy_value.GetType() == footstone::value::HippyValue::Type::kString);
   EXPECT_TRUE(hippy_value.ToStringChecked() == value);
   EXPECT_TRUE(hippy_value.ToStringChecked().length() == value.length());
@@ -108,6 +112,7 @@ void CheckMap(footstone::value::HippyValue::HippyValueObjectType value) {
 
   footstone::value::HippyValue hippy_value;
   deserializer.ReadObject(hippy_value);
+  footstone::value::SerializerHelper::DestroyBuffer(buffer);
   EXPECT_TRUE(hippy_value.GetType() == footstone::value::HippyValue::Type::kObject);
   EXPECT_TRUE(hippy_value.IsObject());
   EXPECT_TRUE(hippy_value.ToObjectChecked().size() == value.size());
@@ -134,6 +139,7 @@ void CheckArray(footstone::value::HippyValue::HippyValueArrayType value) {
 
   footstone::value::HippyValue hippy_value;
   deserializer.ReadObject(hippy_value);
+  footstone::value::SerializerHelper::DestroyBuffer(buffer);
   EXPECT_TRUE(hippy_value.GetType() == footstone::value::HippyValue::Type::kArray);
   EXPECT_TRUE(hippy_value.IsArray());
   EXPECT_TRUE(hippy_value.ToArrayChecked().size() == value.size());
@@ -157,6 +163,7 @@ TEST(DeserializerTest, ReadHeader) {
   footstone::value::Deserializer deserializer(buffer.first, buffer.second);
   deserializer.ReadHeader();
   EXPECT_EQ(deserializer.version_, tdf::base::kLatestVersion);
+  footstone::value::SerializerHelper::DestroyBuffer(buffer);
 }
 
 TEST(DeserializerTest, Uint32) {

diff --git a/dom/src/dom/dom_argument.cc b/dom/src/dom/dom_argument.cc
@@ -65,6 +65,7 @@ bool DomArgument::ConvertObjectToBson(const footstone::value::HippyValue& hippy_
   std::pair<uint8_t*, size_t> pair = serializer.Release();
   bson.resize(pair.second);
   memcpy(&bson[0], pair.first, sizeof(uint8_t) * pair.second);
+  footstone::value::SerializerHelper::DestroyBuffer(pair);
   return true;
 }
 

diff --git a/dom/src/dom/dom_manager.cc b/dom/src/dom/dom_manager.cc
@@ -223,8 +223,10 @@ DomManager::byte_string DomManager::GetSnapShot(const std::shared_ptr<RootNode>&
   Serializer serializer;
   serializer.WriteHeader();
   serializer.WriteValue(HippyValue(array));
-  auto ret = serializer.Release();
-  return {reinterpret_cast<const char*>(ret.first), ret.second};
+  auto buffer_pair = serializer.Release();
+  byte_string bs =  {reinterpret_cast<const char*>(buffer_pair.first), buffer_pair.second};
+  footstone::value::SerializerHelper::DestroyBuffer(buffer_pair);
+  return bs;
 }
 
 bool DomManager::SetSnapShot(const std::shared_ptr<RootNode>& root_node, const byte_string& buffer) {

diff --git a/dom/src/dom/serializer_unittests.cc b/dom/src/dom/serializer_unittests.cc
@@ -1,3 +1,23 @@
+/*
+ * Tencent is pleased to support the open source community by making
+ * Hippy available.
+ *
+ * Copyright (C) 2022 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.
+ */
+
 #define private public
 #define protected public
 
@@ -124,7 +144,7 @@ void CheckString(footstone::value::Serializer& serializer, std::string value, si
 
 TEST(SerializerTest, Release) {
   footstone::value::Serializer serializer;
-  serializer.Release();
+  footstone::value::SerializerHelper(serializer.Release());
   EXPECT_EQ(serializer.buffer_ == nullptr, true) << "Serializer buffer is not equal to nullptr.";
   EXPECT_EQ(serializer.buffer_size_, 0) << "Serializer buffer_size is not equal to 0.";
   EXPECT_EQ(serializer.buffer_capacity_, 0) << "Serializer buffer_capacity is not equal to 0.";

diff --git a/modules/footstone/include/footstone/serializer.h b/modules/footstone/include/footstone/serializer.h
@@ -69,6 +69,15 @@ enum class SerializationTag : uint8_t {
   kEndDenseJSArray = '$',
 };
 
+class SerializerHelper {
+ public:
+  static void DestroyBuffer(const std::pair<uint8_t*, size_t>& pair) {
+    if (pair.first) {
+      free(pair.first);
+    }
+  }
+};
+
 class Serializer {
  public:
   Serializer();
@@ -80,7 +89,11 @@ class Serializer {
 
   void WriteValue(const HippyValue& hippy_value);
 
+  /**
+   * @brief 获取 HippyValue 对应序列化数据，注意 Release 后指针交给外部管理，需要自行释放
+   */
   std::pair<uint8_t*, size_t> Release();
+
  private:
 
   void WriteOddball(Oddball oddball);

diff --git a/renderer/native/android/src/main/cpp/src/renderer/native_render_manager.cc b/renderer/native/android/src/main/cpp/src/renderer/native_render_manager.cc
@@ -124,7 +124,7 @@ void NativeRenderManager::CreateRenderNode(std::weak_ptr<RootNode> root_node,
   }
   uint32_t root_id = root->GetId();
 
-  serializer_->Release();
+  footstone::value::SerializerHelper::DestroyBuffer(serializer_->Release());
   serializer_->WriteHeader();
 
   auto len = nodes.size();
@@ -182,8 +182,8 @@ void NativeRenderManager::CreateRenderNode(std::weak_ptr<RootNode> root_node,
   }
   serializer_->WriteValue(HippyValue(dom_node_array));
   std::pair<uint8_t*, size_t> buffer_pair = serializer_->Release();
-
   CallNativeMethod("createNode", root->GetId(), buffer_pair);
+  footstone::value::SerializerHelper::DestroyBuffer(buffer_pair);
 }
 
 void NativeRenderManager::UpdateRenderNode(std::weak_ptr<RootNode> root_node,
@@ -201,7 +201,7 @@ void NativeRenderManager::UpdateRenderNode(std::weak_ptr<RootNode> root_node,
     }
   }
 
-  serializer_->Release();
+  footstone::value::SerializerHelper::DestroyBuffer(serializer_->Release());
   serializer_->WriteHeader();
 
   auto len = nodes.size();
@@ -242,8 +242,8 @@ void NativeRenderManager::UpdateRenderNode(std::weak_ptr<RootNode> root_node,
   }
   serializer_->WriteValue(HippyValue(dom_node_array));
   std::pair<uint8_t*, size_t> buffer_pair = serializer_->Release();
-
   CallNativeMethod("updateNode", root->GetId(), buffer_pair);
+  footstone::value::SerializerHelper::DestroyBuffer(buffer_pair);
 }
 
 void NativeRenderManager::MoveRenderNode(std::weak_ptr<RootNode> root_node,
@@ -253,7 +253,7 @@ void NativeRenderManager::MoveRenderNode(std::weak_ptr<RootNode> root_node,
     return;
   }
 
-  serializer_->Release();
+  footstone::value::SerializerHelper::DestroyBuffer(serializer_->Release());
   serializer_->WriteHeader();
 
   auto len = nodes.size();
@@ -295,6 +295,7 @@ void NativeRenderManager::MoveRenderNode(std::weak_ptr<RootNode> root_node,
   JNIEnvironment::ClearJEnvException(j_env);
   j_env->DeleteLocalRef(j_buffer);
   j_env->DeleteLocalRef(j_class);
+  footstone::value::SerializerHelper::DestroyBuffer(buffer_pair);
 }
 
 void NativeRenderManager::DeleteRenderNode(std::weak_ptr<RootNode> root_node,
@@ -343,7 +344,7 @@ void NativeRenderManager::UpdateLayout(std::weak_ptr<RootNode> root_node,
     return;
   }
 
-  serializer_->Release();
+  footstone::value::SerializerHelper::DestroyBuffer(serializer_->Release());
   serializer_->WriteHeader();
 
   auto len = nodes.size();
@@ -367,8 +368,8 @@ void NativeRenderManager::UpdateLayout(std::weak_ptr<RootNode> root_node,
   }
   serializer_->WriteValue(HippyValue(dom_node_array));
   std::pair<uint8_t*, size_t> buffer_pair = serializer_->Release();
-
   CallNativeMethod("updateLayout", root->GetId(), buffer_pair);
+  footstone::value::SerializerHelper::DestroyBuffer(buffer_pair);
 }
 
 void NativeRenderManager::MoveRenderNode(std::weak_ptr<RootNode> root_node,
@@ -637,11 +638,12 @@ void NativeRenderManager::HandleListenerOps(std::weak_ptr<RootNode> root_node,
     return;
   }
 
-  serializer_->Release();
+  footstone::value::SerializerHelper::DestroyBuffer(serializer_->Release());
   serializer_->WriteHeader();
   serializer_->WriteValue(HippyValue(event_listener_ops));
   std::pair<uint8_t*, size_t> buffer_pair = serializer_->Release();
   CallNativeMethod(method_name, root->GetId(), buffer_pair);
+  footstone::value::SerializerHelper::DestroyBuffer(buffer_pair);
 }
 
 void NativeRenderManager::MarkTextDirty(std::weak_ptr<RootNode> weak_root_node, uint32_t node_id) {