commit 3e378967b379afcd559b53d70d00e758f9c2616d
author Wonsik Kim <wonsik@google.com> Thu Jan 05 17:00:02 2017 +0900
committer Wonsik Kim <wonsik@google.com> Mon Jan 23 17:02:38 2017 +0900
tree 75b39002e7f7e7fbb82742ad8640bcaa3ef6407a
parent dffb51f4e35fe93215b025bdf4216f3b411c734b

stagefright: fix unreleased OMX handle

- Ensure OMX handle is freed even if binder death notification comes
first.
- Add DeathRecipient in ResourceManagerService so that it could
handle dead clients properly.

Fix: 28824626
Fix: 34252788
Test: adb shell am instrument -e size small -w 'android.media.cts/android.support.test.runner.AndroidJUnitRunner'
Change-Id: Ifc441a2771b5674749ff65a4520177dda115b292

services/mediaresourcemanager/ResourceManagerService.cpp

diff --git a/services/mediaresourcemanager/ResourceManagerService.cpp b/services/mediaresourcemanager/ResourceManagerService.cpp
index 7346f51..78bb587 100644
--- a/services/mediaresourcemanager/ResourceManagerService.cpp
+++ b/services/mediaresourcemanager/ResourceManagerService.cpp
@@ -34,6 +34,31 @@
 
 namespace android {
 
+namespace {
+
+class DeathNotifier : public IBinder::DeathRecipient {
+public:
+    DeathNotifier(const wp<ResourceManagerService> &service, int pid, int64_t clientId)
+        : mService(service), mPid(pid), mClientId(clientId) {}
+
+    virtual void binderDied(const wp<IBinder> & /* who */) override {
+        // Don't check for pid validity since we know it's already dead.
+        sp<ResourceManagerService> service = mService.promote();
+        if (service == nullptr) {
+            ALOGW("ResourceManagerService is dead as well.");
+            return;
+        }
+        service->removeResource(mPid, mClientId, false);
+    }
+
+private:
+    wp<ResourceManagerService> mService;
+    int mPid;
+    int64_t mClientId;
+};
+
+}  // namespace
+
 template <typename T>
 static String8 getString(const Vector<T> &items) {
     String8 itemsStr;
@@ -214,17 +239,25 @@
     ResourceInfo& info = getResourceInfoForEdit(clientId, client, infos);
     // TODO: do the merge instead of append.
     info.resources.appendVector(resources);
+    if (info.deathNotifier == nullptr) {
+        info.deathNotifier = new DeathNotifier(this, pid, clientId);
+        IInterface::asBinder(client)->linkToDeath(info.deathNotifier);
+    }
     notifyResourceGranted(pid, resources);
 }
 
 void ResourceManagerService::removeResource(int pid, int64_t clientId) {
+    removeResource(pid, clientId, true);
+}
+
+void ResourceManagerService::removeResource(int pid, int64_t clientId, bool checkValid) {
     String8 log = String8::format(
             "removeResource(pid %d, clientId %lld)",
             pid, (long long) clientId);
     mServiceLog->add(log);
 
     Mutex::Autolock lock(mLock);
-    if (!mProcessInfo->isValidPid(pid)) {
+    if (checkValid && !mProcessInfo->isValidPid(pid)) {
         ALOGE("Rejected removeResource call with invalid pid.");
         return;
     }
@@ -237,6 +270,7 @@
     ResourceInfos &infos = mMap.editValueAt(index);
     for (size_t j = 0; j < infos.size(); ++j) {
         if (infos[j].clientId == clientId) {
+            IInterface::asBinder(infos[j].client)->unlinkToDeath(infos[j].deathNotifier);
             j = infos.removeAt(j);
             found = true;
             break;

services/mediaresourcemanager/ResourceManagerService.h

diff --git a/services/mediaresourcemanager/ResourceManagerService.h b/services/mediaresourcemanager/ResourceManagerService.h
index 2a4a6b2..9e97ac0 100644
--- a/services/mediaresourcemanager/ResourceManagerService.h
+++ b/services/mediaresourcemanager/ResourceManagerService.h
@@ -36,6 +36,7 @@
 struct ResourceInfo {
     int64_t clientId;
     sp<IResourceManagerClient> client;
+    sp<IBinder::DeathRecipient> deathNotifier;
     Vector<MediaResource> resources;
 };
 
@@ -70,6 +71,8 @@
     // Returns true if any resource has been reclaimed, otherwise returns false.
     virtual bool reclaimResource(int callingPid, const Vector<MediaResource> &resources);
 
+    void removeResource(int pid, int64_t clientId, bool checkValid);
+
 protected:
     virtual ~ResourceManagerService();