Dali::Vector release data more safe (Dali::Vector::Replace) 87/266687/6
authorEunki, Hong <eunkiki.hong@samsung.com>
Wed, 17 Nov 2021 07:44:38 +0000 (16:44 +0900)
committerEunki, Hong <eunkiki.hong@samsung.com>
Mon, 22 Nov 2021 05:53:04 +0000 (14:53 +0900)
We got some issue when someone use mData
during Dali::Vector is on reserving.

In this case, mData = nullptr (or mData memory is deleted.) very short time.

Now, we add VectorBase::Replace(void* newData) function.
It mean, VectorBase::mData will be replaced as newData.

It can block thread integrity issue.

Note : VectorBase::Copy can have nullptr between Release() <-> Reserve()
There is no way to fix it without any additional memory.
So just keep current implement logic.

Change-Id: Ie09b1bc00146f4c3be9294853a32c093b3597fba
Signed-off-by: Eunki, Hong <eunkiki.hong@samsung.com>
dali/public-api/common/dali-vector.cpp
dali/public-api/common/dali-vector.h

index 4f153aa..80b03a4 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2021 Samsung Electronics Co., Ltd.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -53,6 +53,26 @@ void VectorBase::Release()
   }
 }
 
+void VectorBase::Replace(void* newData)
+{
+  if(mData)
+  {
+    // adjust pointer to real beginning
+    SizeType* metadata = reinterpret_cast<SizeType*>(mData);
+
+    // Cause timming issue, we set new data before metadata delete.
+    mData = newData;
+
+    // delete metadata address after mData setup safety.
+    delete[](metadata - 2u);
+  }
+  else
+  {
+    // mData was nullptr. Just copy data address
+    mData = newData;
+  }
+}
+
 void VectorBase::SetCount(SizeType count)
 {
   // someone can call Resize( 0u ) before ever populating the vector
@@ -84,10 +104,9 @@ void VectorBase::Reserve(SizeType capacity, SizeType elementSize)
     {
       // copy over the old data
       memcpy(metaData, mData, oldCount * elementSize);
-      // release old buffer
-      Release();
     }
-    mData = metaData;
+    // release old buffer and set new data as mData
+    Replace(reinterpret_cast<void*>(metaData));
   }
 }
 
index fd4565e..80c6364 100644 (file)
@@ -190,6 +190,17 @@ protected: // for Derived classes
    */
   void CopyMemory(char* destination, const char* source, size_t numberOfBytes);
 
+  /**
+   * @brief Replace the data as new data address.
+   * After replace, release the old data.
+   *
+   * It will be used when we want to keep the mData integrity.
+   *
+   * Does not call destructors on objects held.
+   * @param[in] newData new data address to be replaced
+   */
+  void Replace(void* newData);
+
 private:
   // not copyable as it does not know the size of elements
   VectorBase(const VectorBase&) = delete;            ///< Deleted copy constructor. @SINCE_1_0.0
@@ -472,12 +483,8 @@ public: // API
   {
     if(this != &vector)
     {
-      if(VectorBase::mData)
-      {
-        Release();
-      }
-      VectorBase::mData = vector.mData;
-      vector.mData      = nullptr;
+      VectorAlgorithms<BaseType>::Replace(vector.mData);
+      vector.mData = nullptr;
     }
     return *this;
   }