Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 483a2def authored by Steven Moreland's avatar Steven Moreland
Browse files

libutils: RefBase: extra check for double own 

Bug: 232557259
Test: libutils_test
Change-Id: Ibd400750e7973600ec3fa493838a3b52cafe3add
parent ccb1ce32

libutils/RefBase.cpp

+18 −12
Original line number Diff line number Diff line
@@ -744,11 +744,13 @@ RefBase::~RefBase() 
        if (mRefs->mWeak.load(std::memory_order_relaxed) == 0) {

            delete mRefs;

        }

    } else if (mRefs->mStrong.load(std::memory_order_relaxed) == INITIAL_STRONG_VALUE) {

    } else {

        int32_t strongs = mRefs->mStrong.load(std::memory_order_relaxed);



        if (strongs == INITIAL_STRONG_VALUE) {

            // We never acquired a strong reference on this object.



        // TODO: make this fatal, but too much code in Android manages RefBase with

        // new/delete manually (or using other mechanisms such as std::make_unique).

            // It would be nice to make this fatal, but many places use RefBase on the stack.

            // However, this is dangerous because it's also common for code to use the

            // sp<T>(T*) constructor, assuming that if the object is around, it is already

            // owned by an sp<>.
@@ -759,6 +761,10 @@ RefBase::~RefBase() 
#if CALLSTACK_ENABLED

            CallStack::logStack(LOG_TAG);

#endif

        } else if (strongs != 0) {

            LOG_ALWAYS_FATAL("RefBase: object %p with strong count %d deleted. Double owned?", this,

                             strongs);

        }

    }

    // For debugging purposes, clear mRefs.  Ineffective against outstanding wp's.

    const_cast<weakref_impl*&>(mRefs) = nullptr;

libutils/RefBase_test.cpp

+10 −0
Original line number Diff line number Diff line
@@ -265,6 +265,16 @@ TEST(RefBase, AssertWeakRefExistsDeath) { 
    delete foo;

}



TEST(RefBase, DoubleOwnershipDeath) {

    bool isDeleted;

    auto foo = sp<Foo>::make(&isDeleted);



    // if something else thinks it owns foo, should die

    EXPECT_DEATH(delete foo.get(), "");



    EXPECT_FALSE(isDeleted);

}



// Set up a situation in which we race with visit2AndRremove() to delete

// 2 strong references.  Bar destructor checks that there are no early

// deletions and prior updates are visible to destructor.