mediaresourcemanager: add pid to removeResource method

Bug: 22496209
Change-Id: I73311573e8d1ac15fec668a9ef6e6af7a07a1d30

7 files changed, 50 insertions, 46 deletions

diff --git a/include/media/IResourceManagerService.h b/include/media/IResourceManagerService.h
index 067392c..1e4f6de 100644
--- a/include/media/IResourceManagerService.h
+++ b/include/media/IResourceManagerService.h
@@ -43,7 +43,7 @@ public:
             const sp<IResourceManagerClient> client,
             const Vector<MediaResource> &resources) = 0;
 
-    virtual void removeResource(int64_t clientId) = 0;
+    virtual void removeResource(int pid, int64_t clientId) = 0;
 
     virtual bool reclaimResource(
             int callingPid,
diff --git a/include/media/stagefright/MediaCodec.h b/include/media/stagefright/MediaCodec.h
index 09cbe8f..b1dc278 100644
--- a/include/media/stagefright/MediaCodec.h
+++ b/include/media/stagefright/MediaCodec.h
@@ -260,18 +260,18 @@ private:
         virtual void binderDied(const wp<IBinder>& /*who*/);
 
         void addResource(
-                int pid,
                 int64_t clientId,
                 const sp<IResourceManagerClient> client,
                 const Vector<MediaResource> &resources);
 
         void removeResource(int64_t clientId);
 
-        bool reclaimResource(int callingPid, const Vector<MediaResource> &resources);
+        bool reclaimResource(const Vector<MediaResource> &resources);
 
     private:
         Mutex mLock;
         sp<IResourceManagerService> mService;
+        int mPid;
     };
 
     State mState;
diff --git a/media/libmedia/IResourceManagerService.cpp b/media/libmedia/IResourceManagerService.cpp
index 6902e99..4598686 100644
--- a/media/libmedia/IResourceManagerService.cpp
+++ b/media/libmedia/IResourceManagerService.cpp
@@ -85,9 +85,10 @@ public:
         remote()->transact(ADD_RESOURCE, data, &reply);
     }
 
-    virtual void removeResource(int64_t clientId) {
+    virtual void removeResource(int pid, int64_t clientId) {
         Parcel data, reply;
         data.writeInterfaceToken(IResourceManagerService::getInterfaceDescriptor());
+        data.writeInt32(pid);
         data.writeInt64(clientId);
 
         remote()->transact(REMOVE_RESOURCE, data, &reply);
@@ -139,8 +140,9 @@ status_t BnResourceManagerService::onTransact(
 
         case REMOVE_RESOURCE: {
             CHECK_INTERFACE(IResourceManagerService, data, reply);
+            int pid = data.readInt32();
             int64_t clientId = data.readInt64();
-            removeResource(clientId);
+            removeResource(pid, clientId);
             return NO_ERROR;
         } break;
 
diff --git a/media/libstagefright/MediaCodec.cpp b/media/libstagefright/MediaCodec.cpp
index fb32d3a..09742a4 100644
--- a/media/libstagefright/MediaCodec.cpp
+++ b/media/libstagefright/MediaCodec.cpp
@@ -54,10 +54,6 @@
 
 namespace android {
 
-static inline int getCallingPid() {
-    return IPCThreadState::self()->getCallingPid();
-}
-
 static int64_t getId(sp<IResourceManagerClient> client) {
     return (int64_t) client.get();
 }
@@ -108,7 +104,8 @@ private:
     DISALLOW_EVIL_CONSTRUCTORS(ResourceManagerClient);
 };
 
-MediaCodec::ResourceManagerServiceProxy::ResourceManagerServiceProxy() {
+MediaCodec::ResourceManagerServiceProxy::ResourceManagerServiceProxy()
+        : mPid(IPCThreadState::self()->getCallingPid()) {
 }
 
 MediaCodec::ResourceManagerServiceProxy::~ResourceManagerServiceProxy() {
@@ -135,7 +132,6 @@ void MediaCodec::ResourceManagerServiceProxy::binderDied(const wp<IBinder>& /*wh
 }
 
 void MediaCodec::ResourceManagerServiceProxy::addResource(
-        int pid,
         int64_t clientId,
         const sp<IResourceManagerClient> client,
         const Vector<MediaResource> &resources) {
@@ -143,7 +139,7 @@ void MediaCodec::ResourceManagerServiceProxy::addResource(
     if (mService == NULL) {
         return;
     }
-    mService->addResource(pid, clientId, client, resources);
+    mService->addResource(mPid, clientId, client, resources);
 }
 
 void MediaCodec::ResourceManagerServiceProxy::removeResource(int64_t clientId) {
@@ -151,16 +147,16 @@ void MediaCodec::ResourceManagerServiceProxy::removeResource(int64_t clientId) {
     if (mService == NULL) {
         return;
     }
-    mService->removeResource(clientId);
+    mService->removeResource(mPid, clientId);
 }
 
 bool MediaCodec::ResourceManagerServiceProxy::reclaimResource(
-        int callingPid, const Vector<MediaResource> &resources) {
+        const Vector<MediaResource> &resources) {
     Mutex::Autolock _l(mLock);
     if (mService == NULL) {
         return false;
     }
-    return mService->reclaimResource(callingPid, resources);
+    return mService->reclaimResource(mPid, resources);
 }
 
 // static
@@ -375,7 +371,7 @@ status_t MediaCodec::init(const AString &name, bool nameIsType, bool encoder) {
     for (int i = 0; i <= kMaxRetry; ++i) {
         if (i > 0) {
             // Don't try to reclaim resource for the first time.
-            if (!mResourceManagerService->reclaimResource(getCallingPid(), resources)) {
+            if (!mResourceManagerService->reclaimResource(resources)) {
                 break;
             }
         }
@@ -438,7 +434,7 @@ status_t MediaCodec::configure(
     for (int i = 0; i <= kMaxRetry; ++i) {
         if (i > 0) {
             // Don't try to reclaim resource for the first time.
-            if (!mResourceManagerService->reclaimResource(getCallingPid(), resources)) {
+            if (!mResourceManagerService->reclaimResource(resources)) {
                 break;
             }
         }
@@ -517,7 +513,7 @@ void MediaCodec::addResource(const String8 &type, const String8 &subtype, uint64
     Vector<MediaResource> resources;
     resources.push_back(MediaResource(type, subtype, value));
     mResourceManagerService->addResource(
-            getCallingPid(), getId(mResourceManagerClient), mResourceManagerClient, resources);
+            getId(mResourceManagerClient), mResourceManagerClient, resources);
 }
 
 status_t MediaCodec::start() {
@@ -535,7 +531,7 @@ status_t MediaCodec::start() {
     for (int i = 0; i <= kMaxRetry; ++i) {
         if (i > 0) {
             // Don't try to reclaim resource for the first time.
-            if (!mResourceManagerService->reclaimResource(getCallingPid(), resources)) {
+            if (!mResourceManagerService->reclaimResource(resources)) {
                 break;
             }
             // Recover codec from previous error before retry start.
diff --git a/services/mediaresourcemanager/ResourceManagerService.cpp b/services/mediaresourcemanager/ResourceManagerService.cpp
index 61147ff..e54cc5a 100644
--- a/services/mediaresourcemanager/ResourceManagerService.cpp
+++ b/services/mediaresourcemanager/ResourceManagerService.cpp
@@ -179,23 +179,24 @@ void ResourceManagerService::addResource(
     info.resources.appendVector(resources);
 }
 
-void ResourceManagerService::removeResource(int64_t clientId) {
-    String8 log = String8::format("removeResource(%lld)", (long long) clientId);
+void ResourceManagerService::removeResource(int pid, int64_t clientId) {
+    String8 log = String8::format(
+            "removeResource(pid %d, clientId %lld)",
+            pid, (long long) clientId);
     mServiceLog->add(log);
 
     Mutex::Autolock lock(mLock);
+    ssize_t index = mMap.indexOfKey(pid);
+    if (index < 0) {
+        ALOGV("removeResource: didn't find pid %d for clientId %lld", pid, (long long) clientId);
+        return;
+    }
     bool found = false;
-    for (size_t i = 0; i < mMap.size(); ++i) {
-        ResourceInfos &infos = mMap.editValueAt(i);
-        for (size_t j = 0; j < infos.size();) {
-            if (infos[j].clientId == clientId) {
-                j = infos.removeAt(j);
-                found = true;
-            } else {
-                ++j;
-            }
-        }
-        if (found) {
+    ResourceInfos &infos = mMap.editValueAt(index);
+    for (size_t j = 0; j < infos.size(); ++j) {
+        if (infos[j].clientId == clientId) {
+            j = infos.removeAt(j);
+            found = true;
             break;
         }
     }
diff --git a/services/mediaresourcemanager/ResourceManagerService.h b/services/mediaresourcemanager/ResourceManagerService.h
index ca218fc..4769373 100644
--- a/services/mediaresourcemanager/ResourceManagerService.h
+++ b/services/mediaresourcemanager/ResourceManagerService.h
@@ -63,7 +63,7 @@ public:
             const sp<IResourceManagerClient> client,
             const Vector<MediaResource> &resources);
 
-    virtual void removeResource(int64_t clientId);
+    virtual void removeResource(int pid, int64_t clientId);
 
     // Tries to reclaim resource from processes with lower priority than the calling process
     // according to the requested resources.
diff --git a/services/mediaresourcemanager/test/ResourceManagerService_test.cpp b/services/mediaresourcemanager/test/ResourceManagerService_test.cpp
index 8ae6a55..df49ddc 100644
--- a/services/mediaresourcemanager/test/ResourceManagerService_test.cpp
+++ b/services/mediaresourcemanager/test/ResourceManagerService_test.cpp
@@ -29,6 +29,10 @@
 
 namespace android {
 
+static int64_t getId(sp<IResourceManagerClient> client) {
+    return (int64_t) client.get();
+}
+
 struct TestProcessInfo : public ProcessInfoInterface {
     TestProcessInfo() {}
     virtual ~TestProcessInfo() {}
@@ -45,12 +49,12 @@ private:
 };
 
 struct TestClient : public BnResourceManagerClient {
-    TestClient(sp<ResourceManagerService> service)
-        : mReclaimed(false), mService(service) {}
+    TestClient(int pid, sp<ResourceManagerService> service)
+        : mReclaimed(false), mPid(pid), mService(service) {}
 
     virtual bool reclaimResource() {
         sp<IResourceManagerClient> client(this);
-        mService->removeResource((int64_t) client.get());
+        mService->removeResource(mPid, (int64_t) client.get());
         mReclaimed = true;
         return true;
     }
@@ -72,6 +76,7 @@ protected:
 
 private:
     bool mReclaimed;
+    int mPid;
     sp<ResourceManagerService> mService;
     DISALLOW_EVIL_CONSTRUCTORS(TestClient);
 };
@@ -87,9 +92,9 @@ class ResourceManagerServiceTest : public ::testing::Test {
 public:
     ResourceManagerServiceTest()
         : mService(new ResourceManagerService(new TestProcessInfo)),
-          mTestClient1(new TestClient(mService)),
-          mTestClient2(new TestClient(mService)),
-          mTestClient3(new TestClient(mService)) {
+          mTestClient1(new TestClient(kTestPid1, mService)),
+          mTestClient2(new TestClient(kTestPid2, mService)),
+          mTestClient3(new TestClient(kTestPid2, mService)) {
     }
 
 protected:
@@ -144,24 +149,24 @@ protected:
         // kTestPid1 mTestClient1
         Vector<MediaResource> resources1;
         resources1.push_back(MediaResource(String8(kResourceSecureCodec), 1));
-        mService->addResource(kTestPid1, (int64_t) mTestClient1.get(), mTestClient1, resources1);
+        mService->addResource(kTestPid1, getId(mTestClient1), mTestClient1, resources1);
         resources1.push_back(MediaResource(String8(kResourceGraphicMemory), 200));
         Vector<MediaResource> resources11;
         resources11.push_back(MediaResource(String8(kResourceGraphicMemory), 200));
-        mService->addResource(kTestPid1, (int64_t) mTestClient1.get(), mTestClient1, resources11);
+        mService->addResource(kTestPid1, getId(mTestClient1), mTestClient1, resources11);
 
         // kTestPid2 mTestClient2
         Vector<MediaResource> resources2;
         resources2.push_back(MediaResource(String8(kResourceNonSecureCodec), 1));
         resources2.push_back(MediaResource(String8(kResourceGraphicMemory), 300));
-        mService->addResource(kTestPid2, (int64_t) mTestClient2.get(), mTestClient2, resources2);
+        mService->addResource(kTestPid2, getId(mTestClient2), mTestClient2, resources2);
 
         // kTestPid2 mTestClient3
         Vector<MediaResource> resources3;
-        mService->addResource(kTestPid2, (int64_t) mTestClient3.get(), mTestClient3, resources3);
+        mService->addResource(kTestPid2, getId(mTestClient3), mTestClient3, resources3);
         resources3.push_back(MediaResource(String8(kResourceSecureCodec), 1));
         resources3.push_back(MediaResource(String8(kResourceGraphicMemory), 100));
-        mService->addResource(kTestPid2, (int64_t) mTestClient3.get(), mTestClient3, resources3);
+        mService->addResource(kTestPid2, getId(mTestClient3), mTestClient3, resources3);
 
         const PidResourceInfosMap &map = mService->mMap;
         EXPECT_EQ(2u, map.size());
@@ -213,7 +218,7 @@ protected:
     void testRemoveResource() {
         addResource();
 
-        mService->removeResource((int64_t) mTestClient2.get());
+        mService->removeResource(kTestPid2, getId(mTestClient2));
 
         const PidResourceInfosMap &map = mService->mMap;
         EXPECT_EQ(2u, map.size());
@@ -431,7 +436,7 @@ protected:
             verifyClients(true /* c1 */, false /* c2 */, false /* c3 */);
 
             // clean up client 3 which still left
-            mService->removeResource((int64_t) mTestClient3.get());
+            mService->removeResource(kTestPid2, getId(mTestClient3));
         }
     }