ACPR/android_system_core
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Merge "libutils: RefBase: extra check for double own" am: b0fe01da56 am: 414bab91eb am: 1908c24773 am: 5448e26c6e am: 5074f0a45e

Browse source 
Original change: https://android-review.googlesource.com/c/platform/system/core/+/2166103

Change-Id: I0d41f2b0ff656f689bf563d5f40e720c65f5a5b3
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>
This commit is contained in:
Steven Moreland 2022-07-26 23:04:01 +00:00 • committed by Automerger Merge Worker
commit 96bf1a34dd
2 changed files with 27 additions and 11 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

28
libutils/RefBase.cpp
View file

@ -744,21 +744,27 @@ 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) {
// We never acquired a strong reference on this object.
} else {
int32_t strongs = mRefs->mStrong.load(std::memory_order_relaxed);
// 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).
// 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<>.
ALOGW("RefBase: Explicit destruction, weak count = %d (in %p). Use sp<> to manage this "
"object.",
mRefs->mWeak.load(), this);
if (strongs == INITIAL_STRONG_VALUE) {
// We never acquired a strong reference on this object.
// 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<>.
ALOGW("RefBase: Explicit destruction, weak count = %d (in %p). Use sp<> to manage this "
"object.",
mRefs->mWeak.load(), this);
#if CALLSTACK_ENABLED
CallStack::logStack(LOG_TAG);
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;

10
libutils/RefBase_test.cpp
View file

@ -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.