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

Commit dc43fb27 authored by Steven Moreland's avatar Steven Moreland
Browse files

libutils: wp::fromExisting bugfix 

This API was tested before, but it wasn't used until it is needed in
libbinder. Previously it passed the tests because wp::operator==
compares weak_ptrs which are never deleted, but it doesn't check the
value of m_ptr as well. This assumes that the RefBase implementation is
self-consistent.

Future considerations:
- add additional CHECK (perf?)
- add an additional optional CHECK?
- update all refbase tests to use an embellished form of this operator

Bug: 184190315
Test: libutils_test, boot and kill process when libbinder is using this
API

Change-Id: I66c97386d769529d5efae48e06775d4b4a344025
parent 1d685488

libutils/RefBase_test.cpp

+2 −2
Original line number Diff line number Diff line
@@ -242,12 +242,12 @@ TEST(RefBase, ReplacedComparison) { 
}



TEST(RefBase, AssertWeakRefExistsSuccess) {

    // uses some other refcounting method, or non at all

    bool isDeleted;

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

    wp<Foo> weakFoo = foo;



    EXPECT_EQ(weakFoo, wp<Foo>::fromExisting(foo.get()));

    EXPECT_EQ(weakFoo.unsafe_get(), wp<Foo>::fromExisting(foo.get()).unsafe_get());



    EXPECT_FALSE(isDeleted);

    foo = nullptr;
@@ -255,7 +255,7 @@ TEST(RefBase, AssertWeakRefExistsSuccess) { 
}



TEST(RefBase, AssertWeakRefExistsDeath) {

    // uses some other refcounting method, or non at all

    // uses some other refcounting method, or none at all

    bool isDeleted;

    Foo* foo = new Foo(&isDeleted);

libutils/include/utils/RefBase.h

+1 −0
Original line number Diff line number Diff line
@@ -547,6 +547,7 @@ wp<T> wp<T>::fromExisting(T* other) { 
    refs->incWeakRequireWeak(other);



    wp<T> ret;

    ret.m_ptr = other;

    ret.m_refs = refs;

    return ret;

}