Merge changes I9d3a049f,I0d52ed5e,I556435ca
am: fea8cb29b9
Change-Id: I301b32b3558f2370d81d2fa31d7f9c00d74a1b75
diff --git a/libs/binder/ndk/AIBinder.cpp b/libs/binder/ndk/AIBinder.cpp
index 2219f8e..c02a77a 100644
--- a/libs/binder/ndk/AIBinder.cpp
+++ b/libs/binder/ndk/AIBinder.cpp
@@ -22,6 +22,8 @@
#include <android-base/logging.h>
+using DeathRecipient = ::android::IBinder::DeathRecipient;
+
using ::android::IBinder;
using ::android::Parcel;
using ::android::sp;
@@ -32,10 +34,10 @@
static const void* kId = "ABBinder";
static void* kValue = static_cast<void*>(new bool{true});
-void cleanId(const void* /*id*/, void* /*obj*/, void* /*cookie*/){/* do nothing */};
+void clean(const void* /*id*/, void* /*obj*/, void* /*cookie*/){/* do nothing */};
static void attach(const sp<IBinder>& binder) {
- binder->attachObject(kId, kValue, nullptr /*cookie*/, cleanId);
+ binder->attachObject(kId, kValue, nullptr /*cookie*/, clean);
}
static bool has(const sp<IBinder>& binder) {
return binder != nullptr && binder->findObject(kId) == kValue;
@@ -43,6 +45,21 @@
} // namespace ABBinderTag
+namespace ABpBinderTag {
+
+static std::mutex gLock;
+static const void* kId = "ABpBinder";
+struct Value {
+ wp<ABpBinder> binder;
+};
+void clean(const void* id, void* obj, void* cookie) {
+ CHECK(id == kId) << id << " " << obj << " " << cookie;
+
+ delete static_cast<Value*>(obj);
+};
+
+} // namespace ABpBinderTag
+
AIBinder::AIBinder(const AIBinder_Class* clazz) : mClazz(clazz) {}
AIBinder::~AIBinder() {}
@@ -121,14 +138,51 @@
}
ABpBinder::~ABpBinder() {}
-sp<AIBinder> ABpBinder::fromBinder(const ::android::sp<::android::IBinder>& binder) {
+void ABpBinder::onLastStrongRef(const void* id) {
+ {
+ std::lock_guard<std::mutex> lock(ABpBinderTag::gLock);
+ // Since ABpBinder is OBJECT_LIFETIME_WEAK, we must remove this weak reference in order for
+ // the ABpBinder to be deleted. Since a strong reference to this ABpBinder object should no
+ // longer be able to exist at the time of this method call, there is no longer a need to
+ // recover it.
+
+ ABpBinderTag::Value* value =
+ static_cast<ABpBinderTag::Value*>(remote()->findObject(ABpBinderTag::kId));
+ if (value != nullptr) {
+ value->binder = nullptr;
+ }
+ }
+
+ BpRefBase::onLastStrongRef(id);
+}
+
+sp<AIBinder> ABpBinder::lookupOrCreateFromBinder(const ::android::sp<::android::IBinder>& binder) {
if (binder == nullptr) {
return nullptr;
}
if (ABBinderTag::has(binder)) {
return static_cast<ABBinder*>(binder.get());
}
- return new ABpBinder(binder);
+
+ // The following code ensures that for a given binder object (remote or local), if it is not an
+ // ABBinder then at most one ABpBinder object exists in a given process representing it.
+ std::lock_guard<std::mutex> lock(ABpBinderTag::gLock);
+
+ ABpBinderTag::Value* value =
+ static_cast<ABpBinderTag::Value*>(binder->findObject(ABpBinderTag::kId));
+ if (value == nullptr) {
+ value = new ABpBinderTag::Value;
+ binder->attachObject(ABpBinderTag::kId, static_cast<void*>(value), nullptr /*cookie*/,
+ ABpBinderTag::clean);
+ }
+
+ sp<ABpBinder> ret = value->binder.promote();
+ if (ret == nullptr) {
+ ret = new ABpBinder(binder);
+ value->binder = ret;
+ }
+
+ return ret;
}
struct AIBinder_Weak {
@@ -179,6 +233,63 @@
return new AIBinder_Class(interfaceDescriptor, onCreate, onDestroy, onTransact);
}
+void AIBinder_DeathRecipient::TransferDeathRecipient::binderDied(const wp<IBinder>& who) {
+ CHECK(who == mWho);
+
+ mOnDied(mCookie);
+ mWho = nullptr;
+}
+
+AIBinder_DeathRecipient::AIBinder_DeathRecipient(AIBinder_DeathRecipient_onBinderDied onDied)
+ : mOnDied(onDied) {
+ CHECK(onDied != nullptr);
+}
+
+binder_status_t AIBinder_DeathRecipient::linkToDeath(AIBinder* binder, void* cookie) {
+ CHECK(binder != nullptr);
+
+ std::lock_guard<std::mutex> l(mDeathRecipientsMutex);
+
+ sp<TransferDeathRecipient> recipient =
+ new TransferDeathRecipient(binder->getBinder(), cookie, mOnDied);
+
+ binder_status_t status = binder->getBinder()->linkToDeath(recipient, cookie, 0 /*flags*/);
+ if (status != EX_NONE) {
+ return status;
+ }
+
+ mDeathRecipients.push_back(recipient);
+ return EX_NONE;
+}
+
+binder_status_t AIBinder_DeathRecipient::unlinkToDeath(AIBinder* binder, void* cookie) {
+ CHECK(binder != nullptr);
+
+ std::lock_guard<std::mutex> l(mDeathRecipientsMutex);
+
+ for (auto it = mDeathRecipients.rbegin(); it != mDeathRecipients.rend(); ++it) {
+ sp<TransferDeathRecipient> recipient = *it;
+
+ if (recipient->getCookie() == cookie &&
+
+ recipient->getWho() == binder->getBinder()) {
+ mDeathRecipients.erase(it.base() - 1);
+
+ binder_status_t status =
+ binder->getBinder()->unlinkToDeath(recipient, cookie, 0 /*flags*/);
+ if (status != EX_NONE) {
+ LOG(ERROR) << __func__
+ << ": removed reference to death recipient but unlink failed.";
+ }
+ return status;
+ }
+ }
+
+ return -ENOENT;
+}
+
+// start of C-API methods
+
AIBinder* AIBinder_new(const AIBinder_Class* clazz, void* args) {
if (clazz == nullptr) {
LOG(ERROR) << __func__ << ": Must provide class to construct local binder.";
@@ -218,6 +329,26 @@
return binder->getBinder()->pingBinder();
}
+binder_status_t AIBinder_linkToDeath(AIBinder* binder, AIBinder_DeathRecipient* recipient,
+ void* cookie) {
+ if (binder == nullptr || recipient == nullptr) {
+ LOG(ERROR) << __func__ << ": Must provide binder and recipient.";
+ return EX_NULL_POINTER;
+ }
+
+ return recipient->linkToDeath(binder, cookie);
+}
+
+binder_status_t AIBinder_unlinkToDeath(AIBinder* binder, AIBinder_DeathRecipient* recipient,
+ void* cookie) {
+ if (binder == nullptr || recipient == nullptr) {
+ LOG(ERROR) << __func__ << ": Must provide binder and recipient.";
+ return EX_NULL_POINTER;
+ }
+
+ return recipient->unlinkToDeath(binder, cookie);
+}
+
void AIBinder_incStrong(AIBinder* binder) {
if (binder == nullptr) {
LOG(ERROR) << __func__ << ": on null binder";
@@ -347,3 +478,21 @@
return parcelStatus;
}
+
+AIBinder_DeathRecipient* AIBinder_DeathRecipient_new(
+ AIBinder_DeathRecipient_onBinderDied onBinderDied) {
+ if (onBinderDied == nullptr) {
+ LOG(ERROR) << __func__ << ": requires non-null onBinderDied parameter.";
+ return nullptr;
+ }
+ return new AIBinder_DeathRecipient(onBinderDied);
+}
+
+void AIBinder_DeathRecipient_delete(AIBinder_DeathRecipient** recipient) {
+ if (recipient == nullptr) {
+ return;
+ }
+
+ delete *recipient;
+ *recipient = nullptr;
+}
diff --git a/libs/binder/ndk/AIBinder_internal.h b/libs/binder/ndk/AIBinder_internal.h
index 23949bb..30009d2 100644
--- a/libs/binder/ndk/AIBinder_internal.h
+++ b/libs/binder/ndk/AIBinder_internal.h
@@ -20,6 +20,8 @@
#include "AIBinder_internal.h"
#include <atomic>
+#include <mutex>
+#include <vector>
#include <binder/Binder.h>
#include <binder/IBinder.h>
@@ -79,13 +81,18 @@
void* mUserData;
};
-// This binder object may be remote or local (even though it is 'Bp'). It is not yet associated with
-// a class.
+// 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 {
- static ::android::sp<AIBinder> fromBinder(const ::android::sp<::android::IBinder>& binder);
+ // 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(
+ const ::android::sp<::android::IBinder>& binder);
virtual ~ABpBinder();
+ void onLastStrongRef(const void* id) override;
+
::android::sp<::android::IBinder> getBinder() override { return remote(); }
ABpBinder* asABpBinder() override { return this; }
@@ -108,3 +115,38 @@
// one.
const ::android::String16 mInterfaceDescriptor;
};
+
+// Ownership is like this (when linked to death):
+//
+// AIBinder_DeathRecipient -sp-> TransferDeathRecipient <-wp-> IBinder
+//
+// When the AIBinder_DeathRecipient is dropped, so are the actual underlying death recipients. When
+// the IBinder dies, only a wp to it is kept.
+struct AIBinder_DeathRecipient {
+ // One of these is created for every linkToDeath. This is to be able to recover data when a
+ // binderDied receipt only gives us information about the IBinder.
+ struct TransferDeathRecipient : ::android::IBinder::DeathRecipient {
+ TransferDeathRecipient(const ::android::wp<::android::IBinder>& who, void* cookie,
+ const AIBinder_DeathRecipient_onBinderDied& onDied)
+ : mWho(who), mCookie(cookie), mOnDied(onDied) {}
+
+ void binderDied(const ::android::wp<::android::IBinder>& who) override;
+
+ const ::android::wp<::android::IBinder>& getWho() { return mWho; }
+ void* getCookie() { return mCookie; }
+
+ private:
+ ::android::wp<::android::IBinder> mWho;
+ void* mCookie;
+ const AIBinder_DeathRecipient_onBinderDied& mOnDied;
+ };
+
+ AIBinder_DeathRecipient(AIBinder_DeathRecipient_onBinderDied onDied);
+ binder_status_t linkToDeath(AIBinder* binder, void* cookie);
+ binder_status_t unlinkToDeath(AIBinder* binder, void* cookie);
+
+private:
+ std::mutex mDeathRecipientsMutex;
+ std::vector<::android::sp<TransferDeathRecipient>> mDeathRecipients;
+ AIBinder_DeathRecipient_onBinderDied mOnDied;
+};
diff --git a/libs/binder/ndk/AParcel.cpp b/libs/binder/ndk/AParcel.cpp
index 93384d6..f090929 100644
--- a/libs/binder/ndk/AParcel.cpp
+++ b/libs/binder/ndk/AParcel.cpp
@@ -35,7 +35,8 @@
}
binder_status_t AParcel_writeStrongBinder(AParcel* parcel, AIBinder* binder) {
- return (*parcel)->writeStrongBinder(binder->getBinder());
+ sp<IBinder> writeBinder = binder != nullptr ? binder->getBinder() : nullptr;
+ return (*parcel)->writeStrongBinder(writeBinder);
}
binder_status_t AParcel_readStrongBinder(const AParcel* parcel, AIBinder** binder) {
sp<IBinder> readBinder = nullptr;
@@ -43,7 +44,7 @@
if (status != EX_NONE) {
return status;
}
- sp<AIBinder> ret = ABpBinder::fromBinder(readBinder);
+ sp<AIBinder> ret = ABpBinder::lookupOrCreateFromBinder(readBinder);
AIBinder_incStrong(ret.get());
*binder = ret.get();
return status;
@@ -54,7 +55,7 @@
if (status != EX_NONE) {
return status;
}
- sp<AIBinder> ret = ABpBinder::fromBinder(readBinder);
+ sp<AIBinder> ret = ABpBinder::lookupOrCreateFromBinder(readBinder);
AIBinder_incStrong(ret.get());
*binder = ret.get();
return status;
diff --git a/libs/binder/ndk/AServiceManager.cpp b/libs/binder/ndk/AServiceManager.cpp
index 3979945..90dd1c8 100644
--- a/libs/binder/ndk/AServiceManager.cpp
+++ b/libs/binder/ndk/AServiceManager.cpp
@@ -41,7 +41,7 @@
sp<IServiceManager> sm = defaultServiceManager();
sp<IBinder> binder = sm->getService(String16(instance));
- sp<AIBinder> ret = ABpBinder::fromBinder(binder);
+ sp<AIBinder> ret = ABpBinder::lookupOrCreateFromBinder(binder);
AIBinder_incStrong(ret.get());
return ret.get();
}
diff --git a/libs/binder/ndk/include_ndk/android/binder_ibinder.h b/libs/binder/ndk/include_ndk/android/binder_ibinder.h
index 65bc5a5..7dca5a4 100644
--- a/libs/binder/ndk/include_ndk/android/binder_ibinder.h
+++ b/libs/binder/ndk/include_ndk/android/binder_ibinder.h
@@ -94,6 +94,16 @@
* implementation (usually a callback) to transfer all ownership to a remote process and
* automatically be deleted when the remote process is done with it or dies. Other memory models are
* possible, but this is the standard one.
+ *
+ * If the process containing an AIBinder dies, it is possible to be holding a strong reference to
+ * an object which does not exist. In this case, transactions to this binder will return
+ * EX_DEAD_OBJECT. See also AIBinder_linkToDeath, AIBinder_unlinkToDeath, and AIBinder_isAlive.
+ *
+ * Once an AIBinder is created, anywhere it is passed (remotely or locally), there is a 1-1
+ * correspondence between the address of an AIBinder and the object it represents. This means that
+ * when two AIBinder pointers point to the same address, they represent the same object (whether
+ * that object is local or remote). This correspondance can be broken accidentally if AIBinder_new
+ * is erronesouly called to create the same object multiple times.
*/
struct AIBinder;
typedef struct AIBinder AIBinder;
@@ -107,6 +117,12 @@
typedef struct AIBinder_Weak AIBinder_Weak;
/**
+ * Represents a handle on a death notification. See AIBinder_linkToDeath/AIBinder_unlinkToDeath.
+ */
+struct AIBinder_DeathRecipient;
+typedef struct AIBinder_DeathRecipient AIBinder_DeathRecipient;
+
+/**
* This is called whenever a new AIBinder object is needed of a specific class.
*
* These arguments are passed from AIBinder_new. The return value is stored and can be retrieved
@@ -148,6 +164,14 @@
*
* When this is called, the refcount is implicitly 1. So, calling decStrong exactly one time is
* required to delete this object.
+ *
+ * Once an AIBinder object is created using this API, re-creating that AIBinder for the same
+ * instance of the same class will break pointer equality for that specific AIBinder object. For
+ * instance, if someone erroneously created two AIBinder instances representing the same callback
+ * object and passed one to a hypothetical addCallback function and then later another one to a
+ * hypothetical removeCallback function, the remote process would have no way to determine that
+ * these two objects are actually equal using the AIBinder pointer alone (which they should be able
+ * to do). Also see the suggested memory ownership model suggested above.
*/
__attribute__((warn_unused_result)) AIBinder* AIBinder_new(const AIBinder_Class* clazz, void* args);
@@ -173,6 +197,26 @@
binder_status_t AIBinder_ping(AIBinder* binder);
/**
+ * Registers for notifications that the associated binder is dead. The same death recipient may be
+ * associated with multiple different binders. If the binder is local, then no death recipient will
+ * be given (since if the local process dies, then no recipient will exist to recieve a
+ * transaction). The cookie is passed to recipient in the case that this binder dies and can be
+ * null. The exact cookie must also be used to unlink this transaction (see AIBinder_linkToDeath).
+ * This function may return a binder transaction failure. The cookie can be used both for
+ * identification and holding user data.
+ */
+binder_status_t AIBinder_linkToDeath(AIBinder* binder, AIBinder_DeathRecipient* recipient,
+ void* cookie);
+
+/**
+ * Stops registration for the associated binder dying. Does not delete the recipient. This function
+ * may return a binder transaction failure and in case the death recipient cannot be found, it
+ * returns -ENOENT.
+ */
+binder_status_t AIBinder_unlinkToDeath(AIBinder* binder, AIBinder_DeathRecipient* recipient,
+ void* cookie);
+
+/**
* This can only be called if a strong reference to this object already exists in process.
*/
void AIBinder_incStrong(AIBinder* binder);
@@ -264,6 +308,23 @@
*/
__attribute__((warn_unused_result)) AIBinder* AIBinder_Weak_promote(AIBinder_Weak* weakBinder);
+/**
+ * This function is executed on death receipt. See AIBinder_linkToDeath/AIBinder_unlinkToDeath.
+ */
+typedef void (*AIBinder_DeathRecipient_onBinderDied)(void* cookie);
+
+/**
+ * Creates a new binder death recipient. This can be attached to multiple different binder objects.
+ */
+__attribute__((warn_unused_result)) AIBinder_DeathRecipient* AIBinder_DeathRecipient_new(
+ AIBinder_DeathRecipient_onBinderDied onBinderDied);
+
+/**
+ * Deletes a binder death recipient. It is not necessary to call AIBinder_unlinkToDeath before
+ * calling this as these will all be automatically unlinked.
+ */
+void AIBinder_DeathRecipient_delete(AIBinder_DeathRecipient** recipient);
+
__END_DECLS
/** @} */
diff --git a/libs/binder/ndk/test/main_client.cpp b/libs/binder/ndk/test/main_client.cpp
index 967789f..b8518d7 100644
--- a/libs/binder/ndk/test/main_client.cpp
+++ b/libs/binder/ndk/test/main_client.cpp
@@ -16,6 +16,7 @@
#include <android-base/logging.h>
#include <android/binder_manager.h>
+#include <android/binder_process.h>
#include <gtest/gtest.h>
#include <iface/iface.h>
@@ -45,6 +46,28 @@
AIBinder_decStrong(binder);
}
+void OnBinderDeath(void* cookie) {
+ LOG(ERROR) << "BINDER DIED. COOKIE: " << cookie;
+}
+
+TEST(NdkBinder, LinkToDeath) {
+ ABinderProcess_setThreadPoolMaxThreadCount(1); // to recieve death notifications
+ ABinderProcess_startThreadPool();
+
+ AIBinder* binder = AServiceManager_getService(kExistingNonNdkService);
+ ASSERT_NE(nullptr, binder);
+
+ AIBinder_DeathRecipient* recipient = AIBinder_DeathRecipient_new(OnBinderDeath);
+ ASSERT_NE(nullptr, recipient);
+
+ EXPECT_EQ(EX_NONE, AIBinder_linkToDeath(binder, recipient, nullptr));
+ EXPECT_EQ(EX_NONE, AIBinder_unlinkToDeath(binder, recipient, nullptr));
+ EXPECT_EQ(-ENOENT, AIBinder_unlinkToDeath(binder, recipient, nullptr));
+
+ AIBinder_DeathRecipient_delete(&recipient);
+ AIBinder_decStrong(binder);
+}
+
class MyTestFoo : public IFoo {
int32_t doubleNumber(int32_t in) override {
LOG(INFO) << "doubleNumber " << in;
@@ -64,6 +87,34 @@
EXPECT_EQ(2, getFoo->doubleNumber(1));
}
+TEST(NdkBinder, EqualityOfRemoteBinderPointer) {
+ AIBinder* binderA = AServiceManager_getService(kExistingNonNdkService);
+ ASSERT_NE(nullptr, binderA);
+
+ AIBinder* binderB = AServiceManager_getService(kExistingNonNdkService);
+ ASSERT_NE(nullptr, binderB);
+
+ EXPECT_EQ(binderA, binderB);
+
+ AIBinder_decStrong(binderA);
+ AIBinder_decStrong(binderB);
+}
+
+TEST(NdkBinder, ABpBinderRefCount) {
+ AIBinder* binder = AServiceManager_getService(kExistingNonNdkService);
+ AIBinder_Weak* wBinder = AIBinder_Weak_new(binder);
+
+ ASSERT_NE(nullptr, binder);
+ EXPECT_EQ(1, AIBinder_debugGetRefCount(binder));
+
+ AIBinder_decStrong(binder);
+
+ // assert because would need to decStrong if non-null and we shouldn't need to add a no-op here
+ ASSERT_NE(nullptr, AIBinder_Weak_promote(wBinder));
+
+ AIBinder_Weak_delete(&wBinder);
+}
+
TEST(NdkBinder, AddServiceMultipleTimes) {
static const char* kInstanceName1 = "test-multi-1";
static const char* kInstanceName2 = "test-multi-2";