libbinder_ndk: eradicate global lock
Let the nightmares cease!
Without contention, peace.
Note, in the case that an ABpBinder is backed by a BBinder (this happens
when you talk in-process to something between the NDK backend and either
the C++, Rust, or Java backends of AIDL), this introduces an additional
allocation b/c the BBinder's mExtra needs to be allocated for
BBinder::withLock to work. Since this is generally frowned upon, and the
thrashing in this case is already pretty egregious, I don't mind it.
Now, binder proxies can be allocated simultaneously in a process.
Bug: 192023359
Test: CtsNdkBinderTestCases
Change-Id: Ib1c28c9488f7a03579ea9d18347a8fc08cc3f48a
diff --git a/libs/binder/ndk/ibinder.cpp b/libs/binder/ndk/ibinder.cpp
index eb44683..266ef37 100644
--- a/libs/binder/ndk/ibinder.cpp
+++ b/libs/binder/ndk/ibinder.cpp
@@ -57,7 +57,6 @@
namespace ABpBinderTag {
-static std::mutex gLock;
static const void* kId = "ABpBinder";
struct Value {
wp<ABpBinder> binder;
@@ -252,23 +251,28 @@
return static_cast<ABBinder*>(binder.get());
}
- // 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));
+ auto* 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);
+ auto oldValue = static_cast<ABpBinderTag::Value*>(
+ binder->attachObject(ABpBinderTag::kId, static_cast<void*>(value),
+ nullptr /*cookie*/, ABpBinderTag::clean));
+
+ // allocated by another thread
+ if (oldValue) {
+ delete value;
+ value = oldValue;
+ }
}
- sp<ABpBinder> ret = value->binder.promote();
- if (ret == nullptr) {
- ret = new ABpBinder(binder);
- value->binder = ret;
- }
+ sp<ABpBinder> ret;
+ binder->withLock([&]() {
+ ret = value->binder.promote();
+ if (ret == nullptr) {
+ ret = sp<ABpBinder>::make(binder);
+ value->binder = ret;
+ }
+ });
return ret;
}