From: Eunki, Hong Date: Thu, 6 Oct 2022 07:41:21 +0000 (+0900) Subject: Make Dali::Any move operator + Resolve memory leak. X-Git-Tag: dali_2.1.44~1^2 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=2a88f17f81f28012b9c0b6d2fd7479d37cd4b1f7;p=platform%2Fcore%2Fuifw%2Fdali-core.git 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 --- diff --git a/automated-tests/src/dali/utc-Dali-Any.cpp b/automated-tests/src/dali/utc-Dali-Any.cpp index 39bc2f4ec..eed09ef0a 100644 --- 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 d03d07dfe..ad5111ea3 100644 --- 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 997c318ec..7a58c7401 100644 --- 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. @@ -100,6 +100,19 @@ 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. * @@ -107,7 +120,6 @@ public: * @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 Any& operator=(const Type& value) @@ -142,10 +154,18 @@ 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. * @@ -398,6 +418,7 @@ public: Type mValue; }; +private: AnyContainerBase* mContainer; };