resourcemanager: rework death handling
remove the dependency on static lock and maps
for handling death notifications.
Bug: 289097671
Test: atest android.media.misc.cts.ResourceManagerTest
atest android.media.misc.cts.ResourceManagerMultiTest
/data/nativetest64/ResourceManagerService_test/ResourceManagerService_test
/data/nativetest64/ResourceObserverService_test/ResourceObserverService_test
Change-Id: Iaed7d5141458e31803ffdefa5fee67497d33d7c1
diff --git a/media/libstagefright/MediaCodec.cpp b/media/libstagefright/MediaCodec.cpp
index 19fc1c9..8be9c48 100644
--- a/media/libstagefright/MediaCodec.cpp
+++ b/media/libstagefright/MediaCodec.cpp
@@ -330,7 +330,7 @@
std::scoped_lock lock{mLock};
// Unregistering from DeathRecipient notification.
if (mService != nullptr) {
- AIBinder_unlinkToDeath(mService->asBinder().get(), mDeathRecipient.get(), this);
+ AIBinder_unlinkToDeath(mService->asBinder().get(), mDeathRecipient.get(), mCookie);
mService = nullptr;
}
}
@@ -370,13 +370,15 @@
std::shared_ptr<IResourceManagerClient> mClient;
::ndk::ScopedAIBinder_DeathRecipient mDeathRecipient;
std::shared_ptr<IResourceManagerService> mService;
+ BinderDiedContext* mCookie;
};
MediaCodec::ResourceManagerServiceProxy::ResourceManagerServiceProxy(
pid_t pid, uid_t uid, const std::shared_ptr<IResourceManagerClient> &client) :
mPid(pid), mUid(uid), mClient(client),
mDeathRecipient(::ndk::ScopedAIBinder_DeathRecipient(
- AIBinder_DeathRecipient_new(BinderDiedCallback))) {
+ AIBinder_DeathRecipient_new(BinderDiedCallback))),
+ mCookie(nullptr) {
if (mUid == MediaCodec::kNoUid) {
mUid = AIBinder_getCallingUid();
}
@@ -434,9 +436,9 @@
// Create the context that is passed as cookie to the binder death notification.
// The context gets deleted at BinderUnlinkedCallback.
- BinderDiedContext* context = new BinderDiedContext{.mRMServiceProxy = weak_from_this()};
+ mCookie = new BinderDiedContext{.mRMServiceProxy = weak_from_this()};
// Register for the callbacks by linking to death notification.
- AIBinder_linkToDeath(mService->asBinder().get(), mDeathRecipient.get(), context);
+ AIBinder_linkToDeath(mService->asBinder().get(), mDeathRecipient.get(), mCookie);
// If the RM was restarted, re-register all the resources.
if (mBinderDied) {
diff --git a/services/mediaresourcemanager/ResourceManagerService.cpp b/services/mediaresourcemanager/ResourceManagerService.cpp
index b2971c4..f718406 100644
--- a/services/mediaresourcemanager/ResourceManagerService.cpp
+++ b/services/mediaresourcemanager/ResourceManagerService.cpp
@@ -44,47 +44,82 @@
namespace android {
-//static
-std::mutex ResourceManagerService::sCookieLock;
-//static
-uintptr_t ResourceManagerService::sCookieCounter = 0;
-//static
-std::map<uintptr_t, sp<DeathNotifier> > ResourceManagerService::sCookieToDeathNotifierMap;
+class DeathNotifier : public std::enable_shared_from_this<DeathNotifier> {
-class DeathNotifier : public RefBase {
+ // BinderDiedContext defines the cookie that is passed as DeathRecipient.
+ // Since this can maintain more context than a raw pointer, we can
+ // validate the scope of DeathNotifier, before deferencing it upon the binder death.
+ struct BinderDiedContext {
+ std::weak_ptr<DeathNotifier> mDeathNotifier;
+ };
public:
- DeathNotifier(const std::shared_ptr<ResourceManagerService> &service,
- const ClientInfoParcel& clientInfo);
+ DeathNotifier(const std::shared_ptr<IResourceManagerClient>& client,
+ const std::shared_ptr<ResourceManagerService>& service,
+ const ClientInfoParcel& clientInfo,
+ AIBinder_DeathRecipient* recipient);
- virtual ~DeathNotifier() {}
+ virtual ~DeathNotifier() {
+ unlink();
+ }
+
+ void unlink() {
+ if (mClient != nullptr) {
+ // Register for the callbacks by linking to death notification.
+ AIBinder_unlinkToDeath(mClient->asBinder().get(), mRecipient, mCookie);
+ mClient = nullptr;
+ }
+ }
// Implement death recipient
static void BinderDiedCallback(void* cookie);
+ static void BinderUnlinkedCallback(void* cookie);
virtual void binderDied();
+private:
+ void link() {
+ // Create the context that is passed as cookie to the binder death notification.
+ // The context gets deleted at BinderUnlinkedCallback.
+ mCookie = new BinderDiedContext{.mDeathNotifier = weak_from_this()};
+ // Register for the callbacks by linking to death notification.
+ AIBinder_linkToDeath(mClient->asBinder().get(), mRecipient, mCookie);
+ }
+
protected:
+ std::shared_ptr<IResourceManagerClient> mClient;
std::weak_ptr<ResourceManagerService> mService;
const ClientInfoParcel mClientInfo;
+ AIBinder_DeathRecipient* mRecipient;
+ BinderDiedContext* mCookie;
};
-DeathNotifier::DeathNotifier(const std::shared_ptr<ResourceManagerService> &service,
- const ClientInfoParcel& clientInfo)
- : mService(service), mClientInfo(clientInfo) {}
+DeathNotifier::DeathNotifier(const std::shared_ptr<IResourceManagerClient>& client,
+ const std::shared_ptr<ResourceManagerService>& service,
+ const ClientInfoParcel& clientInfo,
+ AIBinder_DeathRecipient* recipient)
+ : mClient(client), mService(service), mClientInfo(clientInfo),
+ mRecipient(recipient), mCookie(nullptr) {
+ link();
+}
+
+//static
+void DeathNotifier::BinderUnlinkedCallback(void* cookie) {
+ BinderDiedContext* context = reinterpret_cast<BinderDiedContext*>(cookie);
+ // Since we don't need the context anymore, we are deleting it now.
+ delete context;
+}
//static
void DeathNotifier::BinderDiedCallback(void* cookie) {
- sp<DeathNotifier> notifier;
- {
- std::scoped_lock lock{ResourceManagerService::sCookieLock};
- auto it = ResourceManagerService::sCookieToDeathNotifierMap.find(
- reinterpret_cast<uintptr_t>(cookie));
- if (it == ResourceManagerService::sCookieToDeathNotifierMap.end()) {
- return;
+ BinderDiedContext* context = reinterpret_cast<BinderDiedContext*>(cookie);
+
+ // Validate the context and check if the DeathNotifier object is still in scope.
+ if (context != nullptr) {
+ std::shared_ptr<DeathNotifier> thiz = context->mDeathNotifier.lock();
+ if (thiz != nullptr) {
+ thiz->binderDied();
+ } else {
+ ALOGI("DeathNotifier is out of scope already");
}
- notifier = it->second;
- }
- if (notifier.get() != nullptr) {
- notifier->binderDied();
}
}
@@ -103,9 +138,11 @@
class OverrideProcessInfoDeathNotifier : public DeathNotifier {
public:
- OverrideProcessInfoDeathNotifier(const std::shared_ptr<ResourceManagerService> &service,
- const ClientInfoParcel& clientInfo)
- : DeathNotifier(service, clientInfo) {}
+ OverrideProcessInfoDeathNotifier(const std::shared_ptr<IResourceManagerClient>& client,
+ const std::shared_ptr<ResourceManagerService>& service,
+ const ClientInfoParcel& clientInfo,
+ AIBinder_DeathRecipient* recipient)
+ : DeathNotifier(client, service, clientInfo, recipient) {}
virtual ~OverrideProcessInfoDeathNotifier() {}
@@ -195,7 +232,7 @@
.clientId = clientId,
.name = name.empty()? "<unknown client>" : name,
.client = client,
- .cookie = 0,
+ .deathNotifier = nullptr,
.pendingRemoval = false};
auto [it, inserted] = infos.emplace(clientId, info);
found = it;
@@ -341,7 +378,8 @@
mSupportsMultipleSecureCodecs(true),
mSupportsSecureWithNonSecureCodec(true),
mCpuBoostCount(0),
- mDeathRecipient(AIBinder_DeathRecipient_new(DeathNotifier::BinderDiedCallback)) {
+ mDeathRecipient(::ndk::ScopedAIBinder_DeathRecipient(
+ AIBinder_DeathRecipient_new(DeathNotifier::BinderDiedCallback))) {
mSystemCB->noteResetVideo();
// Create ResourceManagerMetrics that handles all the metrics.
mResourceManagerMetrics = std::make_unique<ResourceManagerMetrics>(mProcessInfo);
@@ -496,9 +534,9 @@
mergeResources(it->second, res);
}
}
- if (info.cookie == 0 && client != nullptr) {
- info.cookie = addCookieAndLink_l(client,
- new DeathNotifier(ref<ResourceManagerService>(), clientInfo));
+ if (info.deathNotifier == nullptr && client != nullptr) {
+ info.deathNotifier = std::make_shared<DeathNotifier>(
+ client, ref<ResourceManagerService>(), clientInfo, mDeathRecipient.get());
}
if (mObserverService != nullptr && !resourceAdded.empty()) {
mObserverService->onResourceAdded(uid, pid, resourceAdded);
@@ -615,8 +653,6 @@
// Since this client has been removed, update the metrics collector.
mResourceManagerMetrics->notifyClientReleased(clientInfo);
- removeCookieAndUnlink_l(info.client, info.cookie);
-
if (mObserverService != nullptr && !info.resources.empty()) {
mObserverService->onResourceRemoved(info.uid, pid, info.resources);
}
@@ -869,40 +905,14 @@
.uid = 0,
.id = 0,
.name = "<unknown client>"};
- uintptr_t cookie = addCookieAndLink_l(client,
- new OverrideProcessInfoDeathNotifier(ref<ResourceManagerService>(), clientInfo));
+ auto deathNotifier = std::make_shared<OverrideProcessInfoDeathNotifier>(
+ client, ref<ResourceManagerService>(), clientInfo, mDeathRecipient.get());
- mProcessInfoOverrideMap.emplace(pid, ProcessInfoOverride{cookie, client});
+ mProcessInfoOverrideMap.emplace(pid, ProcessInfoOverride{deathNotifier, client});
return Status::ok();
}
-uintptr_t ResourceManagerService::addCookieAndLink_l(
- const std::shared_ptr<IResourceManagerClient>& client, const sp<DeathNotifier>& notifier) {
- if (client == nullptr) {
- return 0;
- }
- std::scoped_lock lock{sCookieLock};
-
- uintptr_t cookie;
- // Need to skip cookie 0 (if it wraps around). ResourceInfo has cookie initialized to 0
- // indicating the death notifier is not created yet.
- while ((cookie = ++sCookieCounter) == 0);
- AIBinder_linkToDeath(client->asBinder().get(), mDeathRecipient.get(), (void*)cookie);
- sCookieToDeathNotifierMap.emplace(cookie, notifier);
-
- return cookie;
-}
-
-void ResourceManagerService::removeCookieAndUnlink_l(
- const std::shared_ptr<IResourceManagerClient>& client, uintptr_t cookie) {
- std::scoped_lock lock{sCookieLock};
- if (client != nullptr) {
- AIBinder_unlinkToDeath(client->asBinder().get(), mDeathRecipient.get(), (void*)cookie);
- }
- sCookieToDeathNotifierMap.erase(cookie);
-}
-
void ResourceManagerService::removeProcessInfoOverride(int pid) {
std::scoped_lock lock{mLock};
@@ -916,9 +926,6 @@
}
mProcessInfo->removeProcessInfoOverride(pid);
-
- removeCookieAndUnlink_l(it->second.client, it->second.cookie);
-
mProcessInfoOverrideMap.erase(pid);
}
diff --git a/services/mediaresourcemanager/ResourceManagerService.h b/services/mediaresourcemanager/ResourceManagerService.h
index a05a346..aa88ac6 100644
--- a/services/mediaresourcemanager/ResourceManagerService.h
+++ b/services/mediaresourcemanager/ResourceManagerService.h
@@ -56,7 +56,7 @@
int64_t clientId;
std::string name;
std::shared_ptr<IResourceManagerClient> client;
- uintptr_t cookie{0};
+ std::shared_ptr<DeathNotifier> deathNotifier = nullptr;
ResourceList resources;
bool pendingRemoval{false};
};
@@ -186,10 +186,6 @@
void removeProcessInfoOverride(int pid);
void removeProcessInfoOverride_l(int pid);
- uintptr_t addCookieAndLink_l(const std::shared_ptr<IResourceManagerClient>& client,
- const sp<DeathNotifier>& notifier);
- void removeCookieAndUnlink_l(const std::shared_ptr<IResourceManagerClient>& client,
- uintptr_t cookie);
void pushReclaimAtom(const ClientInfoParcel& clientInfo,
const std::vector<std::shared_ptr<IResourceManagerClient>>& clients,
@@ -210,15 +206,11 @@
int32_t mCpuBoostCount;
::ndk::ScopedAIBinder_DeathRecipient mDeathRecipient;
struct ProcessInfoOverride {
- uintptr_t cookie;
+ std::shared_ptr<DeathNotifier> deathNotifier = nullptr;
std::shared_ptr<IResourceManagerClient> client;
};
std::map<int, int> mOverridePidMap;
std::map<pid_t, ProcessInfoOverride> mProcessInfoOverrideMap;
- static std::mutex sCookieLock;
- static uintptr_t sCookieCounter GUARDED_BY(sCookieLock);
- static std::map<uintptr_t, sp<DeathNotifier> > sCookieToDeathNotifierMap
- GUARDED_BY(sCookieLock);
std::shared_ptr<ResourceObserverService> mObserverService;
std::unique_ptr<ResourceManagerMetrics> mResourceManagerMetrics;
};