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

Commit 3acd490d authored by Steven Moreland's avatar Steven Moreland
Browse files

libbinder_ndk: avoid BpRefBase 

This is OBJECT_LIFETIME_WEAK which makes things
complicated. The goal of this is to be able to
support this kind of thing:

    sp<IFoo> foo = ... // get service;
    wp<IFoo> wFoo = foo;
    foo = nullptr;

    sp<IFoo> foo2 = ... // get service again;
    EXPECT_EQ(wFoo.promote(), foo2);

However, because ABpBinder onLastStrongRef expunged
the wp binder reference, this did not work in the
NDK backend of binder before. This works for BpBinder
because BpBinder does not call expungeHandle until
~BpBinder. However, because we need the reference
from our BpBinder to the BpRefBase, it becomes
circular. I'm not sure, with the current
implementation of RefBase, it would be possible
to get this behavior. In the future, if we wanted
it, we could make all BpBinder objects also be
ABpBinder objects. This would avoid the extra
allocation and avoid the need for the proxy object.

Bug: 220141324
Test: CtsNdkBinderTestCases

Change-Id: I934a12baec625da1c3a0d5052d446523dc4c1f89
parent dcc3db7b

libs/binder/ndk/ibinder.cpp

+4 −16
Original line number Diff line number Diff line
@@ -64,6 +64,9 @@ struct Value { 
    wp<ABpBinder> binder;

};

void clean(const void* id, void* obj, void* cookie) {

    // be weary of leaks!

    // LOG(INFO) << "Deleting an ABpBinder";



    CHECK(id == kId) << id << " " << obj << " " << cookie;



    delete static_cast<Value*>(obj);
@@ -239,26 +242,11 @@ status_t ABBinder::onTransact(transaction_code_t code, const Parcel& data, Parce 
}



ABpBinder::ABpBinder(const ::android::sp<::android::IBinder>& binder)

    : AIBinder(nullptr /*clazz*/), BpRefBase(binder) {

    : AIBinder(nullptr /*clazz*/), mRemote(binder) {

    CHECK(binder != nullptr);

}

ABpBinder::~ABpBinder() {}



void ABpBinder::onLastStrongRef(const void* id) {

    // Since ABpBinder is OBJECT_LIFETIME_WEAK, we must remove this weak reference in order for

    // the ABpBinder to be deleted. Even though we have no more references on the ABpBinder

    // (BpRefBase), the remote object may still exist (for instance, if we

    // receive it from another process, before the ABpBinder is attached).



    ABpBinderTag::Value* value =

            static_cast<ABpBinderTag::Value*>(remote()->findObject(ABpBinderTag::kId));

    CHECK_NE(nullptr, value) << "ABpBinder must always be attached";



    remote()->withLock([&]() { value->binder = nullptr; });



    BpRefBase::onLastStrongRef(id);

}



sp<AIBinder> ABpBinder::lookupOrCreateFromBinder(const ::android::sp<::android::IBinder>& binder) {

    if (binder == nullptr) {

        return nullptr;

libs/binder/ndk/ibinder_internal.h

+3 −4
Original line number Diff line number Diff line
@@ -91,7 +91,7 @@ struct ABBinder : public AIBinder, public ::android::BBinder { 


// This binder object may be remote or local (even though it is 'Bp'). The implication if it is

// local is that it is an IBinder object created outside of the domain of libbinder_ndk.

struct ABpBinder : public AIBinder, public ::android::BpRefBase {

struct ABpBinder : public AIBinder {

    // Looks up to see if this object has or is an existing ABBinder or ABpBinder object, otherwise

    // it creates an ABpBinder object.

    static ::android::sp<AIBinder> lookupOrCreateFromBinder(
@@ -99,14 +99,13 @@ struct ABpBinder : public AIBinder, public ::android::BpRefBase { 


    virtual ~ABpBinder();



    void onLastStrongRef(const void* id) override;



    ::android::sp<::android::IBinder> getBinder() override { return remote(); }

    ::android::sp<::android::IBinder> getBinder() override { return mRemote; }

    ABpBinder* asABpBinder() override { return this; }



   private:

    friend android::sp<ABpBinder>;

    explicit ABpBinder(const ::android::sp<::android::IBinder>& binder);

    ::android::sp<::android::IBinder> mRemote;

};



struct AIBinder_Class {