Make Dali::Any move operator + Resolve memory leak.

Coverity minor issue fixed.
+
Resolve some memory leak.

Due to Dali::Any copy assign operator doesn't call
previous object's destructor, there can be memory leak.
(For example : Property::Map delete some values in destructor.)

Change-Id: Ifa29dd78b4f3689ed0ff00d9b8bcfe8ad9e167d6
Signed-off-by: Eunki, Hong <eunkiki.hong@samsung.com>

diff --git a/automated-tests/src/dali/utc-Dali-Any.cpp b/automated-tests/src/dali/utc-Dali-Any.cpp
index 39bc2f4..eed09ef 100644 (file)
--- a/automated-tests/src/dali/utc-Dali-Any.cpp
+++ b/automated-tests/src/dali/utc-Dali-Any.cpp
@@ -26,24 +26,33 @@
 
 namespace
 {
+static int gRefCount;
 struct MyStruct
 {
   MyStruct()
   : mFloatValue(0.f),
     mIntValue(0)
   {
+    ++gRefCount;
   }
 
   MyStruct(float fValue, int iValue)
   : mFloatValue(fValue),
     mIntValue(iValue)
   {
+    ++gRefCount;
   }
 
   MyStruct(const MyStruct& myStruct)
   : mFloatValue(myStruct.mFloatValue),
     mIntValue(myStruct.mIntValue)
   {
+    ++gRefCount;
+  }
+
+  ~MyStruct()
+  {
+    --gRefCount;
   }
 
   MyStruct& operator=(const MyStruct& myStruct)
@@ -77,6 +86,8 @@ int UtcDaliAnyConstructors(void)
 
   tet_infoline("Test Any constructors.");
 
+  gRefCount = 0;
+
   // Test default constructor.
   Any value;
 
@@ -91,9 +102,21 @@ int UtcDaliAnyConstructors(void)
   // Test constructor Any( const Any& ) with a non initialized Any
   Any value3 = value;
 
+  // Test constructor Any( Any&& ) with a non initialized Any
+  Any value4(Any(MyStruct(1.0f, 2)));
+
   DALI_TEST_CHECK(typeid(unsigned int) == value1.GetType());
   DALI_TEST_CHECK(typeid(unsigned int) == value2.GetType());
   DALI_TEST_CHECK(typeid(void) == value3.GetType());
+  DALI_TEST_CHECK(typeid(MyStruct) == value4.GetType());
+  DALI_TEST_CHECK(gRefCount == 1);
+
+  // Test std::move operation result.
+  Any value5(std::move(value4));
+
+  DALI_TEST_CHECK(value4.Empty());
+  DALI_TEST_CHECK(typeid(MyStruct) == value5.GetType());
+  DALI_TEST_CHECK(gRefCount == 1);
 
   unsigned int uiValue1 = 0u;
   unsigned int uiValue2 = 0u;
@@ -101,6 +124,13 @@ int UtcDaliAnyConstructors(void)
   value2.Get(uiValue2);
 
   DALI_TEST_EQUALS(uiValue1, uiValue2, TEST_LOCATION);
+
+  MyStruct myValue;
+  value5.Get(myValue);
+
+  DALI_TEST_EQUALS(myValue.mFloatValue, 1.0f, Math::MACHINE_EPSILON_1000, TEST_LOCATION);
+  DALI_TEST_EQUALS(myValue.mIntValue, 2, TEST_LOCATION);
+
   END_TEST;
 }
 
@@ -162,11 +192,45 @@ int UtcDaliAnyAssignmentOperators(void)
   value6.Get(fValue);
   DALI_TEST_EQUALS(fValue, 3.f, Math::MACHINE_EPSILON_1000, TEST_LOCATION);
 
-  // test assignment for  non-empty Any = empty Any
+  // test assignment for non-empty Any = empty Any
   Any value7;
   value6 = value7;
   DALI_TEST_CHECK(value6.Empty());
 
+  // Due to value6 is reference of value5, value5 also become empty
+  DALI_TEST_CHECK(value5.Empty());
+
+  gRefCount = 0;
+
+  // Do something to above compiler optimize out
+  Any value8 = value3;
+
+  DALI_TEST_CHECK(typeid(float) == value8.GetType());
+
+  // Test operator=( Any&& ).
+  value8 = Any(MyStruct(3.0f, 4));
+
+  DALI_TEST_CHECK(typeid(MyStruct) == value8.GetType());
+  DALI_TEST_CHECK(gRefCount == 1);
+
+  // Do something to above compiler optimize out
+  Any value9 = value3;
+
+  DALI_TEST_CHECK(typeid(float) == value9.GetType());
+
+  // Test std::move operation result.
+  value9 = std::move(value8);
+
+  DALI_TEST_CHECK(value8.Empty());
+  DALI_TEST_CHECK(typeid(MyStruct) == value9.GetType());
+  DALI_TEST_CHECK(gRefCount == 1);
+
+  MyStruct myValue;
+  value9.Get(myValue);
+
+  DALI_TEST_EQUALS(myValue.mFloatValue, 3.0f, Math::MACHINE_EPSILON_1000, TEST_LOCATION);
+  DALI_TEST_EQUALS(myValue.mIntValue, 4, TEST_LOCATION);
+
   END_TEST;
 }
 
@@ -355,3 +419,40 @@ int UtcDaliAnyNegativeGet(void)
   uiValue++; // supresss warning from unused variable
   END_TEST;
 }
+
+int UtcDaliAnyReferenceCheck(void)
+{
+  gRefCount = 0;
+
+  {
+    Dali::Any any[10]; // Create local 10 empty Any
+
+    DALI_TEST_EQUALS(gRefCount, 0, TEST_LOCATION);
+
+    // Create [0 5)
+    for(int i = 0; i < 5; i++)
+    {
+      any[i] = MyStruct(1.0f, i);
+    }
+    DALI_TEST_EQUALS(gRefCount, 5, TEST_LOCATION);
+
+    // Move from [0 5) to [5 10)
+    for(int i = 0; i < 5; i++)
+    {
+      any[i + 5] = std::move(any[i]);
+    }
+    DALI_TEST_EQUALS(gRefCount, 5, TEST_LOCATION);
+
+    // Copy from [5 10) to [0 5)
+    for(int i = 0; i < 5; i++)
+    {
+      any[i] = any[i + 5];
+    }
+    DALI_TEST_EQUALS(gRefCount, 10, TEST_LOCATION);
+  }
+
+  // Check whether all Dali::Any are released
+  DALI_TEST_EQUALS(gRefCount, 0, TEST_LOCATION);
+
+  END_TEST;
+}
\ No newline at end of file

diff --git a/dali/public-api/object/any.cpp b/dali/public-api/object/any.cpp
index d03d07d..ad5111e 100644 (file)
--- a/dali/public-api/object/any.cpp
+++ b/dali/public-api/object/any.cpp
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2020 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2022 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.
@@ -45,8 +45,11 @@ Any& Any::operator=(const Any& any)
   {
     if(nullptr == any.mContainer)
     {
-      delete mContainer;
-      mContainer = nullptr;
+      if(mContainer)
+      {
+        mContainer->mDeleteFunc(mContainer);
+        mContainer = nullptr;
+      }
     }
     else
     {
@@ -65,8 +68,31 @@ Any& Any::operator=(const Any& any)
       mContainer = any.mContainer->mCloneFunc(*any.mContainer);
 
       // Deletes previous container.
-      delete tmp;
+      if(tmp)
+      {
+        tmp->mDeleteFunc(tmp);
+      }
+    }
+  }
+
+  return *this;
+}
+
+Any& Any::operator=(Any&& any) noexcept
+{
+  if(&any != this)
+  {
+    // Deletes previous container.
+    if(mContainer)
+    {
+      mContainer->mDeleteFunc(mContainer);
     }
+
+    // Move the correct templated object
+    mContainer = any.mContainer;
+
+    // Remove input value's container.
+    any.mContainer = nullptr;
   }
 
   return *this;

diff --git a/dali/public-api/object/any.h b/dali/public-api/object/any.h
index 997c318..7a58c74 100644 (file)
--- a/dali/public-api/object/any.h
+++ b/dali/public-api/object/any.h
@@ -2,7 +2,7 @@
 #define DALI_ANY_TYPE_H
 
 /*
- * Copyright (c) 2020 Samsung Electronics Co., Ltd.
+ * Copyright (c) 2022 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.
@@ -101,13 +101,25 @@ public:
   }
 
   /**
+   * @brief Move Constructor.
+   * @SINCE_2_1.44
+   * @param[in] any Any to be moved
+   */
+  Any(Any&& any) noexcept
+  {
+    mContainer = any.mContainer;
+
+    // Remove input value's container.
+    any.mContainer = nullptr;
+  }
+
+  /**
    * @brief Assigns a given value to the Any type.
    *
    * @SINCE_1_0.0
    * @param[in] value The given value
    * @return A reference to this
    * @note If the types are different, then the current container will be re-created.
-   *
    */
   template<typename Type>
   Any& operator=(const Type& value)
@@ -142,11 +154,19 @@ public:
    * @param[in] any Any to be assigned which contains a value of identical type to current contents.
    * @return A reference to this
    * @exception DaliException If parameter any is of a different type.
-   *
    */
   DALI_CORE_API Any& operator=(const Any& any);
 
   /**
+   * @brief Move assignment operator.
+   *
+   * @SINCE_2_1.44
+   * @param[in] any Any to be moved which contains a value of identical type to current contents.
+   * @return A reference to this
+   */
+  DALI_CORE_API Any& operator=(Any&& any) noexcept;
+
+  /**
    * @brief Gets a value of type Type from container.
    *
    * @SINCE_1_0.0
@@ -398,6 +418,7 @@ public:
     Type mValue;
   };
 
+private:
   AnyContainerBase* mContainer;
 };