From b0f0fd66afa048cb05562fbbb4fed0381c987fb3 Mon Sep 17 00:00:00 2001 From: lijincheng Date: Thu, 14 Sep 2023 16:04:12 +0800 Subject: [PATCH] Enhance Refbase's ability to monitor and alert in extreme situations Issue:https://gitee.com/openharmony/commonlibrary_c_utils/issues/I81ELS Signed-off-by: lijincheng --- base/include/refbase.h | 50 ++++++++++++++----- base/src/refbase.cpp | 19 +++++-- .../unittest/common/utils_refbase_test.cpp | 19 +++++++ docs/zh-cn/c-utils-guide-refbase.md | 33 ++++++++++-- 4 files changed, 101 insertions(+), 20 deletions(-) diff --git a/base/include/refbase.h b/base/include/refbase.h index a49bfcc..0a3a86f 100644 --- a/base/include/refbase.h +++ b/base/include/refbase.h @@ -592,11 +592,25 @@ public: ~sptr(); + /** + * @brief Create a new object with class type (T) and provide a new sptr to manage it. + * + * @note We strongly recommend using `sptr::MakeSptr` to create a object and manage it. + * This approach avoids object pointer leaks, which can avoid many potential memory problems. + * + * @return A sptr which manage a new object with class type (T). + * @param args Constructor parameters of the new object to be managed by sptr. + */ + template + static inline sptr MakeSptr(Args&&... args); + /** * @brief Constructor with the specified object to be managed. + * And We do not recommend using this interface to create an sptr object. + * Using the interface `sptr::MakeSptr` is better * * @note A null sptr will be created if `other` is `nullptr`. - * @param other Object to be managed by wptr. + * @param other Object to be managed by sptr. */ sptr(T *other); @@ -680,6 +694,16 @@ public: return refs_; } + /** + * @brief Explicit boolean conversion operator. + * + * @return `true` if refbase object is not a "null ptr"; `false` otherwise. + */ + inline explicit operator bool() const + { + return refs_ != nullptr; + } + /** * @brief Dereference operator. * @@ -703,19 +727,10 @@ public: return refs_; } - /** - * @brief Logical-NOT operator, which is used to check if - * the sptr is a "null sptr". - * - * @return `true` if sptr is a "null sptr"; `false` otherwise. - */ - inline bool operator!() const - { - return refs_ == nullptr; - } - /** * @brief Copy assignment operator with the specified object to be managed. + * And We do not recommend using this interface to create an sptr object. + * Using the interface `sptr::MakeSptr` is better. * * @note The original reference will be removed, and a new reference to the * input object will be established. @@ -823,6 +838,17 @@ private: T *refs_ = nullptr; // Raw pointer to the specific managed object }; +template +template +sptr sptr::MakeSptr(Args&&... args) +{ + T *ptr = new T(std::forward(args)...); + sptr res; + res.ForceSetRefPtr(ptr); + ptr->IncStrongRef(ptr); + return res; +} + template inline void sptr::ForceSetRefPtr(T *other) { diff --git a/base/src/refbase.cpp b/base/src/refbase.cpp index 3b71cf7..ec96997 100644 --- a/base/src/refbase.cpp +++ b/base/src/refbase.cpp @@ -14,8 +14,8 @@ */ #include "refbase.h" -#ifdef DEBUG_REFBASE #include "utils_log.h" +#ifdef DEBUG_REFBASE #include #endif @@ -247,6 +247,7 @@ int RefCounter::DecStrongRefCount(const void* objectId) int curCount = GetStrongRefCount(); if (curCount == INITIAL_PRIMARY_VALUE) { // unexpected case: there had never a strong reference. + UTILS_LOGF("decStrongRef when there is nerver a strong reference"); } else if (curCount > 0) { // we should update the current count here. // it may be changed after last operation. @@ -294,14 +295,15 @@ int RefCounter::DecWeakRefCount(const void* objectId) if (curCount != 1) { return curCount; } - - if (IsLifeTimeExtended() && GetStrongRefCount() == 0) { + std::atomic_thread_fence(std::memory_order_acquire); + if (IsLifeTimeExtended()) { if (callback_) { callback_(); } } else { - // only weak ptr case: no strong reference, delete the object + // only weak ptr but never had a strong ref, we should do nothing here theoretically. But it may cause a leak. if (GetStrongRefCount() == INITIAL_PRIMARY_VALUE) { + UTILS_LOGW("dec the last weakRef before it had a strong reference, delete refbase to avoid Memory Leak"); if (callback_) { callback_(); } @@ -373,7 +375,7 @@ bool RefCounter::AttemptIncStrongRef(const void *objectId, int &outCount) } ATTEMPT_SUCCESS: - if (curCount >= INITIAL_PRIMARY_VALUE) { + if (curCount == INITIAL_PRIMARY_VALUE) { outCount = curCount; atomicStrong_.fetch_sub(INITIAL_PRIMARY_VALUE, std::memory_order_release); return true; @@ -493,6 +495,9 @@ void RefBase::IncStrongRef(const void *objectId) IncWeakRef(objectId); const int curCount = refs_->IncStrongRefCount(objectId); + if (!refs_->IsLifeTimeExtended() && curCount == 0) { + UTILS_LOGF("%{public}p still incStrongRef after last strong ref", this); + } if (curCount == INITIAL_PRIMARY_VALUE) { OnFirstStrongRef(objectId); } @@ -511,7 +516,11 @@ void RefBase::DecStrongRef(const void *objectId) RefCounter * const refs = refs_; const int curCount = refs->DecStrongRefCount(objectId); + if (curCount <= 0) { + UTILS_LOGF("%{public}p call decStrongRef too many times", this); + } if (curCount == 1) { + std::atomic_thread_fence(std::memory_order_acquire); OnLastStrongRef(objectId); if (!refs->IsLifeTimeExtended()) { if (refs->callback_) { diff --git a/base/test/unittest/common/utils_refbase_test.cpp b/base/test/unittest/common/utils_refbase_test.cpp index 5041e41..6672314 100644 --- a/base/test/unittest/common/utils_refbase_test.cpp +++ b/base/test/unittest/common/utils_refbase_test.cpp @@ -658,6 +658,25 @@ HWTEST_F(UtilsRefbaseTest, testSptrefbase010, TestSize.Level0) EXPECT_EQ(testObject1->GetSptrRefCount(), 1); } +/* + * @tc.name: testSptrefbase011 + * @tc.desc: Refbase + */ +HWTEST_F(UtilsRefbaseTest, testSptrefbase011, TestSize.Level0) +{ + sptr testobject = sptr::MakeSptr(); + testobject->ExtendObjectLifetime(); + EXPECT_TRUE(testobject->IsExtendLifeTimeSet()); + EXPECT_EQ(g_refbaseflag, 1); + wptr weakObject(testobject); + int count = testobject->GetWptrRefCount(); + EXPECT_EQ(count, 2); + testobject = nullptr; + + sptr strongObject = weakObject.promote(); + EXPECT_EQ(strongObject->GetSptrRefCount(), 1); +} + class SptrTest : public RefBase { public: SptrTest() diff --git a/docs/zh-cn/c-utils-guide-refbase.md b/docs/zh-cn/c-utils-guide-refbase.md index 194c114..f9c5412 100644 --- a/docs/zh-cn/c-utils-guide-refbase.md +++ b/docs/zh-cn/c-utils-guide-refbase.md @@ -45,17 +45,18 @@ class OHOS::sptr; | 返回类型 | 名称 | | --------------------------------------- | -------------------------------------------------------------------------------- | | | **sptr**() | +|sptr< T > | template
**MakeSptr**(Args&&... args)
构造T类型的被管理对象并创建sptr管控，传递参数args为T类型构造函数所需参数
**注意:强烈建议使用该方法构造sptr并管控对象，可以避免对象指针对外暴露，将对象的生命周期完全处于智能指针的管控之下** | | template
| **sptr**(const sptr< O >& other)
拷贝构造函数，参数与当前sptr具有不同的管理类型(O) | | | **sptr**(const sptr< T >& other)
拷贝构造函数。其以参数指定具体管理对象 | | | **sptr**(sptr< T >&& other)
移动构造函数 | -| | **sptr**(T* other)
构造函数。其以参数指定具体管理对象 | +| | **sptr**(T* other)
构造函数。其以参数指定具体管理对象
**注意: 不建议使用对象指针的形式构造sptr对象，这会导致对象的生命周期不完全在sptr的看护下，很可能误用造成对象提前释放** | | | **sptr**(WeakRefCount* p, bool force)
构造函数。仅用于wptr的promote操作 | | | **~sptr**() | | void | **clear**()
移除当前sptr与所管理对象的引用关系 | | void | **ForceSetRefPtr**(T* other)
强制更改被管理对象指针的指向 | | T* | **GetRefPtr**() const
获取sptr管理对象的指针 | | | **operator T***() const
类型转换运算符 | -| bool | **operator!**() const
逻辑非运算符。检查sptr对象是否为空sptr对象 | +| | **operator bool**() const
布尔类型转换运算符。检查sptr对象是否为空对象 | | bool | **operator!=**(const sptr< T >& other) const
sptr对象间的不等运算符 | | bool | **operator!=**(const T* other) const
sptr对象与裸指针间的不等运算符 | | bool | **operator!=**(const wptr< T >& other) const
sptr对象与wptr间的相等运算符 | @@ -65,7 +66,7 @@ class OHOS::sptr; | sptr< T >& | **operator=**(const sptr< T >& other)
拷贝赋值运算符，参数与当前sptr对象具有相同管理类型 | | sptr< T >& | **operator=**(const wptr< T >& other)
拷贝赋值运算符，参数为一个wptr对象，但与当前sptr对象具有相同管理类型 | | sptr< T >& | **operator=**(sptr< T >&& other)
移动构造运算符 | -| sptr< T >& | **operator=**(T* other)
拷贝赋值运算符，参数为待管理的具体对象 | +| sptr< T >& | **operator=**(T* other)
拷贝赋值运算符，参数为待管理的具体对象
**注意: 不建议以指针赋值的形式创建sptr对象，这会导致对象的生命周期不完全在sptr的看护下，很可能误用造成对象提前释放** | | bool | **operator==**(const sptr< T >& other) const
sptr对象间的相等运算符 | | bool | **operator==**(const T* other) const
sptr对象与裸指针间的相等运算符 | | bool | **operator==**(constwptr< T >& other) const
sptr对象与wptr间的相等运算符 | @@ -308,3 +309,29 @@ sptr s(a); // 裸指针a容易被误delete,造成sptr功能失常 * 易造成误解。 * 因编译器优化程度具有不确定的行为，易造成问题。 + +5. **不建议使用对象指针构造智能指针对象** + + * 外部提前以指针形式释放对象后，继续通过智能指针中使用 + * sptr引用计数为0释放对象，对象指针依旧在外被继续使用 + +```cpp +Refbase *a = new Refbase(arg1, arg2); +sptr sp1 = a; // 不建议，对象指针a暴露在外，存在风险 +sptr sp2(a); // 不建议，对象指针a暴露在外，存在风险 +sptr sp3 = sptr::MakeSptr(arg1, arg2); // 建议，在内部构造Refbase对象，直接交与sptr管控使用 +``` +6. **wptr使用注意** + + * 在未设置**ExtendObjectLifetime**的情况下，wptr不参与被管理对象的生命周期控制，对象生命周期由sptr的引用计数控制，但在极特殊情况下存在例外 + +```cpp +// 由于历史设计原因，可以在sptr不存在的情况下，基于对象指针创建wptr对象。 +// 在未设置ExtendObjectLifetime，且无sptr被创建的特殊少见情况下，为了防止内存泄漏，在wptr引用计数归0时会释放管理对象 +Refbase *a = new Refbase(arg1, arg2); +wptr wp1(a); +wp1 = nullptr; // 弱引用计数归0，对象释放，应避免再次手动释放 + +wptr wp2 = new Refbase(arg1, arg2); +wp2 = nullptr; // 弱引用计数归0，对象释放，这种情况无法手动释放, 如果wptr不能控制对象释放则必然会发生内存泄漏 +``` \ No newline at end of file -- Gitee