diff options
| -rw-r--r-- | libs/binder/ndk/ibinder.cpp | 39 | ||||
| -rw-r--r-- | libs/binder/ndk/ibinder_internal.h | 13 | ||||
| -rw-r--r-- | libs/binder/ndk/tests/iface.cpp | 1 | ||||
| -rw-r--r-- | libs/binder/ndk/tests/include/iface/iface.h | 1 | ||||
| -rw-r--r-- | libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp | 124 |
5 files changed, 174 insertions, 4 deletions
diff --git a/libs/binder/ndk/ibinder.cpp b/libs/binder/ndk/ibinder.cpp index bf7a0ba5f0..e2ede3f52a 100644 --- a/libs/binder/ndk/ibinder.cpp +++ b/libs/binder/ndk/ibinder.cpp @@ -258,11 +258,24 @@ status_t ABBinder::onTransact(transaction_code_t code, const Parcel& data, Parce } } +void ABBinder::addDeathRecipient(const ::android::sp<AIBinder_DeathRecipient>& /* recipient */, + void* /* cookie */) { + LOG_ALWAYS_FATAL("Should not reach this. Can't linkToDeath local binders."); +} + ABpBinder::ABpBinder(const ::android::sp<::android::IBinder>& binder) : AIBinder(nullptr /*clazz*/), mRemote(binder) { LOG_ALWAYS_FATAL_IF(binder == nullptr, "binder == nullptr"); } -ABpBinder::~ABpBinder() {} + +ABpBinder::~ABpBinder() { + for (auto& recip : mDeathRecipients) { + sp<AIBinder_DeathRecipient> strongRecip = recip.recipient.promote(); + if (strongRecip) { + strongRecip->pruneThisTransferEntry(getBinder(), recip.cookie); + } + } +} sp<AIBinder> ABpBinder::lookupOrCreateFromBinder(const ::android::sp<::android::IBinder>& binder) { if (binder == nullptr) { @@ -301,6 +314,12 @@ sp<AIBinder> ABpBinder::lookupOrCreateFromBinder(const ::android::sp<::android:: return ret; } +void ABpBinder::addDeathRecipient(const ::android::sp<AIBinder_DeathRecipient>& recipient, + void* cookie) { + std::lock_guard<std::mutex> l(mDeathRecipientsMutex); + mDeathRecipients.emplace_back(recipient, cookie); +} + struct AIBinder_Weak { wp<AIBinder> binder; }; @@ -426,6 +445,17 @@ AIBinder_DeathRecipient::AIBinder_DeathRecipient(AIBinder_DeathRecipient_onBinde LOG_ALWAYS_FATAL_IF(onDied == nullptr, "onDied == nullptr"); } +void AIBinder_DeathRecipient::pruneThisTransferEntry(const sp<IBinder>& who, void* cookie) { + std::lock_guard<std::mutex> l(mDeathRecipientsMutex); + mDeathRecipients.erase(std::remove_if(mDeathRecipients.begin(), mDeathRecipients.end(), + [&](const sp<TransferDeathRecipient>& tdr) { + auto tdrWho = tdr->getWho(); + return tdrWho != nullptr && tdrWho.promote() == who && + cookie == tdr->getCookie(); + }), + mDeathRecipients.end()); +} + void AIBinder_DeathRecipient::pruneDeadTransferEntriesLocked() { mDeathRecipients.erase(std::remove_if(mDeathRecipients.begin(), mDeathRecipients.end(), [](const sp<TransferDeathRecipient>& tdr) { @@ -554,8 +584,11 @@ binder_status_t AIBinder_linkToDeath(AIBinder* binder, AIBinder_DeathRecipient* return STATUS_UNEXPECTED_NULL; } - // returns binder_status_t - return recipient->linkToDeath(binder->getBinder(), cookie); + binder_status_t ret = recipient->linkToDeath(binder->getBinder(), cookie); + if (ret == STATUS_OK) { + binder->addDeathRecipient(recipient, cookie); + } + return ret; } binder_status_t AIBinder_unlinkToDeath(AIBinder* binder, AIBinder_DeathRecipient* recipient, diff --git a/libs/binder/ndk/ibinder_internal.h b/libs/binder/ndk/ibinder_internal.h index 9d5368f674..f5b738c1ef 100644 --- a/libs/binder/ndk/ibinder_internal.h +++ b/libs/binder/ndk/ibinder_internal.h @@ -51,6 +51,8 @@ struct AIBinder : public virtual ::android::RefBase { ::android::sp<::android::IBinder> binder = const_cast<AIBinder*>(this)->getBinder(); return binder->remoteBinder() != nullptr; } + virtual void addDeathRecipient(const ::android::sp<AIBinder_DeathRecipient>& recipient, + void* cookie) = 0; private: // AIBinder instance is instance of this class for a local object. In order to transact on a @@ -78,6 +80,8 @@ struct ABBinder : public AIBinder, public ::android::BBinder { ::android::status_t dump(int fd, const ::android::Vector<::android::String16>& args) override; ::android::status_t onTransact(uint32_t code, const ::android::Parcel& data, ::android::Parcel* reply, binder_flags_t flags) override; + void addDeathRecipient(const ::android::sp<AIBinder_DeathRecipient>& /* recipient */, + void* /* cookie */) override; private: ABBinder(const AIBinder_Class* clazz, void* userData); @@ -106,12 +110,20 @@ struct ABpBinder : public AIBinder { bool isServiceFuzzing() const { return mServiceFuzzing; } void setServiceFuzzing() { mServiceFuzzing = true; } + void addDeathRecipient(const ::android::sp<AIBinder_DeathRecipient>& recipient, + void* cookie) override; private: friend android::sp<ABpBinder>; explicit ABpBinder(const ::android::sp<::android::IBinder>& binder); ::android::sp<::android::IBinder> mRemote; bool mServiceFuzzing = false; + struct DeathRecipientInfo { + android::wp<AIBinder_DeathRecipient> recipient; + void* cookie; + }; + std::mutex mDeathRecipientsMutex; + std::vector<DeathRecipientInfo> mDeathRecipients; }; struct AIBinder_Class { @@ -183,6 +195,7 @@ struct AIBinder_DeathRecipient : ::android::RefBase { binder_status_t linkToDeath(const ::android::sp<::android::IBinder>&, void* cookie); binder_status_t unlinkToDeath(const ::android::sp<::android::IBinder>& binder, void* cookie); void setOnUnlinked(AIBinder_DeathRecipient_onBinderUnlinked onUnlinked); + void pruneThisTransferEntry(const ::android::sp<::android::IBinder>&, void* cookie); private: // When the user of this API deletes a Bp object but not the death recipient, the diff --git a/libs/binder/ndk/tests/iface.cpp b/libs/binder/ndk/tests/iface.cpp index 3ee36cd8c3..ca927272f8 100644 --- a/libs/binder/ndk/tests/iface.cpp +++ b/libs/binder/ndk/tests/iface.cpp @@ -25,6 +25,7 @@ using ::android::wp; const char* IFoo::kSomeInstanceName = "libbinder_ndk-test-IFoo"; const char* IFoo::kInstanceNameToDieFor = "libbinder_ndk-test-IFoo-to-die"; +const char* IFoo::kInstanceNameToDieFor2 = "libbinder_ndk-test-IFoo-to-die2"; const char* IFoo::kIFooDescriptor = "my-special-IFoo-class"; struct IFoo_Class_Data { diff --git a/libs/binder/ndk/tests/include/iface/iface.h b/libs/binder/ndk/tests/include/iface/iface.h index 0a562f085d..0cdd50b37a 100644 --- a/libs/binder/ndk/tests/include/iface/iface.h +++ b/libs/binder/ndk/tests/include/iface/iface.h @@ -27,6 +27,7 @@ class IFoo : public virtual ::android::RefBase { public: static const char* kSomeInstanceName; static const char* kInstanceNameToDieFor; + static const char* kInstanceNameToDieFor2; static const char* kIFooDescriptor; static AIBinder_Class* kClass; diff --git a/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp b/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp index 966ec959b6..ce63b828cb 100644 --- a/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp +++ b/libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp @@ -536,6 +536,7 @@ TEST(NdkBinder, DeathRecipient) { bool deathReceived = false; std::function<void(void)> onDeath = [&] { + std::unique_lock<std::mutex> lockDeath(deathMutex); std::cerr << "Binder died (as requested)." << std::endl; deathReceived = true; deathCv.notify_one(); @@ -547,6 +548,7 @@ TEST(NdkBinder, DeathRecipient) { bool wasDeathReceivedFirst = false; std::function<void(void)> onUnlink = [&] { + std::unique_lock<std::mutex> lockUnlink(unlinkMutex); std::cerr << "Binder unlinked (as requested)." << std::endl; wasDeathReceivedFirst = deathReceived; unlinkReceived = true; @@ -560,7 +562,6 @@ TEST(NdkBinder, DeathRecipient) { EXPECT_EQ(STATUS_OK, AIBinder_linkToDeath(binder, recipient, static_cast<void*>(cookie))); - // the binder driver should return this if the service dies during the transaction EXPECT_EQ(STATUS_DEAD_OBJECT, foo->die()); foo = nullptr; @@ -579,6 +580,123 @@ TEST(NdkBinder, DeathRecipient) { binder = nullptr; } +TEST(NdkBinder, DeathRecipientDropBinderNoDeath) { + using namespace std::chrono_literals; + + std::mutex deathMutex; + std::condition_variable deathCv; + bool deathReceived = false; + + std::function<void(void)> onDeath = [&] { + std::unique_lock<std::mutex> lockDeath(deathMutex); + std::cerr << "Binder died (as requested)." << std::endl; + deathReceived = true; + deathCv.notify_one(); + }; + + std::mutex unlinkMutex; + std::condition_variable unlinkCv; + bool unlinkReceived = false; + bool wasDeathReceivedFirst = false; + + std::function<void(void)> onUnlink = [&] { + std::unique_lock<std::mutex> lockUnlink(unlinkMutex); + std::cerr << "Binder unlinked (as requested)." << std::endl; + wasDeathReceivedFirst = deathReceived; + unlinkReceived = true; + unlinkCv.notify_one(); + }; + + // keep the death recipient around + ndk::ScopedAIBinder_DeathRecipient recipient(AIBinder_DeathRecipient_new(LambdaOnDeath)); + AIBinder_DeathRecipient_setOnUnlinked(recipient.get(), LambdaOnUnlink); + + { + AIBinder* binder; + sp<IFoo> foo = IFoo::getService(IFoo::kInstanceNameToDieFor2, &binder); + ASSERT_NE(nullptr, foo.get()); + ASSERT_NE(nullptr, binder); + + DeathRecipientCookie* cookie = new DeathRecipientCookie{&onDeath, &onUnlink}; + + EXPECT_EQ(STATUS_OK, + AIBinder_linkToDeath(binder, recipient.get(), static_cast<void*>(cookie))); + // let the sp<IFoo> and AIBinder fall out of scope + AIBinder_decStrong(binder); + binder = nullptr; + } + + { + std::unique_lock<std::mutex> lockDeath(deathMutex); + EXPECT_FALSE(deathCv.wait_for(lockDeath, 100ms, [&] { return deathReceived; })); + EXPECT_FALSE(deathReceived); + } + + { + std::unique_lock<std::mutex> lockUnlink(unlinkMutex); + EXPECT_TRUE(deathCv.wait_for(lockUnlink, 1s, [&] { return unlinkReceived; })); + EXPECT_TRUE(unlinkReceived); + EXPECT_FALSE(wasDeathReceivedFirst); + } +} + +TEST(NdkBinder, DeathRecipientDropBinderOnDied) { + using namespace std::chrono_literals; + + std::mutex deathMutex; + std::condition_variable deathCv; + bool deathReceived = false; + + sp<IFoo> foo; + AIBinder* binder; + std::function<void(void)> onDeath = [&] { + std::unique_lock<std::mutex> lockDeath(deathMutex); + std::cerr << "Binder died (as requested)." << std::endl; + deathReceived = true; + AIBinder_decStrong(binder); + binder = nullptr; + deathCv.notify_one(); + }; + + std::mutex unlinkMutex; + std::condition_variable unlinkCv; + bool unlinkReceived = false; + bool wasDeathReceivedFirst = false; + + std::function<void(void)> onUnlink = [&] { + std::unique_lock<std::mutex> lockUnlink(unlinkMutex); + std::cerr << "Binder unlinked (as requested)." << std::endl; + wasDeathReceivedFirst = deathReceived; + unlinkReceived = true; + unlinkCv.notify_one(); + }; + + ndk::ScopedAIBinder_DeathRecipient recipient(AIBinder_DeathRecipient_new(LambdaOnDeath)); + AIBinder_DeathRecipient_setOnUnlinked(recipient.get(), LambdaOnUnlink); + + foo = IFoo::getService(IFoo::kInstanceNameToDieFor2, &binder); + ASSERT_NE(nullptr, foo.get()); + ASSERT_NE(nullptr, binder); + + DeathRecipientCookie* cookie = new DeathRecipientCookie{&onDeath, &onUnlink}; + EXPECT_EQ(STATUS_OK, AIBinder_linkToDeath(binder, recipient.get(), static_cast<void*>(cookie))); + + EXPECT_EQ(STATUS_DEAD_OBJECT, foo->die()); + + { + std::unique_lock<std::mutex> lockDeath(deathMutex); + EXPECT_TRUE(deathCv.wait_for(lockDeath, 1s, [&] { return deathReceived; })); + EXPECT_TRUE(deathReceived); + } + + { + std::unique_lock<std::mutex> lockUnlink(unlinkMutex); + EXPECT_TRUE(deathCv.wait_for(lockUnlink, 100ms, [&] { return unlinkReceived; })); + EXPECT_TRUE(unlinkReceived); + EXPECT_TRUE(wasDeathReceivedFirst); + } +} + TEST(NdkBinder, RetrieveNonNdkService) { #pragma clang diagnostic push #pragma clang diagnostic ignored "-Wdeprecated-declarations" @@ -958,6 +1076,10 @@ int main(int argc, char* argv[]) { } if (fork() == 0) { prctl(PR_SET_PDEATHSIG, SIGHUP); + return manualThreadPoolService(IFoo::kInstanceNameToDieFor2); + } + if (fork() == 0) { + prctl(PR_SET_PDEATHSIG, SIGHUP); return manualPollingService(IFoo::kSomeInstanceName); } if (fork() == 0) { |