commit 9f626cf9cb178c245ff37c043c0975bd74b49de8
author Brian Lindahl <blindahl@google.com> Mon Apr 25 13:29:59 2022 +0200
committer Brian Lindahl <blindahl@google.com> Wed Apr 27 10:22:32 2022 +0000
tree 64c7710ed91a850ffa39e7902e5b7eab77f2c62a
parent cea6ed113850975a599926484700c52405287d70

Remove dead codecs from ResourceManagerService

When a codec is dead, the resources aren't always removed from
ResourceManagerService, causing MediaCodec to attempt to reclaim
resources continuously without actually doing any reclaiming. Remove the
resources from ResourceManagerService when a dead codec is identified.

Passing 10 iterations on coral-userdebug:
https://android-build.googleplex.com/builds/abtd/run/L09800000954236443

Filed b/230362034 to do further investigation as to why the resources
aren't being removed.

Bug: 229848597
Bug: 228238915
Fixes: 229848597
Fixes: 228238915
Test: atest MediaCodecResourceTest
Change-Id: I81f76742c48d5bc61f89f569a20a1dc54bb9f440

media/libstagefright/MediaCodec.cpp

diff --git a/media/libstagefright/MediaCodec.cpp b/media/libstagefright/MediaCodec.cpp
index 2a75342..5a27362 100644
--- a/media/libstagefright/MediaCodec.cpp
+++ b/media/libstagefright/MediaCodec.cpp
@@ -186,9 +186,12 @@
 // XXX suppress until we get our representation right
 static bool kEmitHistogram = false;
 
+static int64_t getId(IResourceManagerClient const * client) {
+    return (int64_t) client;
+}
 
 static int64_t getId(const std::shared_ptr<IResourceManagerClient> &client) {
-    return (int64_t) client.get();
+    return getId(client.get());
 }
 
 static bool isResourceError(status_t err) {
@@ -205,12 +208,20 @@
 ////////////////////////////////////////////////////////////////////////////////
 
 struct ResourceManagerClient : public BnResourceManagerClient {
-    explicit ResourceManagerClient(MediaCodec* codec) : mMediaCodec(codec) {}
+    explicit ResourceManagerClient(MediaCodec* codec, int32_t pid) :
+            mMediaCodec(codec), mPid(pid) {}
 
     Status reclaimResource(bool* _aidl_return) override {
         sp<MediaCodec> codec = mMediaCodec.promote();
         if (codec == NULL) {
-            // codec is already gone.
+            // Codec is already gone, so remove the resources as well
+            ::ndk::SpAIBinder binder(AServiceManager_getService("media.resource_manager"));
+            std::shared_ptr<IResourceManagerService> service =
+                    IResourceManagerService::fromBinder(binder);
+            if (service == nullptr) {
+                ALOGW("MediaCodec::ResourceManagerClient unable to find ResourceManagerService");
+            }
+            service->removeClient(mPid, getId(this));
             *_aidl_return = true;
             return Status::ok();
         }
@@ -247,6 +258,7 @@
 
 private:
     wp<MediaCodec> mMediaCodec;
+    int32_t mPid;
 
     DISALLOW_EVIL_CONSTRUCTORS(ResourceManagerClient);
 };
@@ -820,7 +832,7 @@
       mGetCodecBase(getCodecBase),
       mGetCodecInfo(getCodecInfo) {
     mResourceManagerProxy = new ResourceManagerServiceProxy(pid, uid,
-            ::ndk::SharedRefBase::make<ResourceManagerClient>(this));
+            ::ndk::SharedRefBase::make<ResourceManagerClient>(this, pid));
     if (!mGetCodecBase) {
         mGetCodecBase = [](const AString &name, const char *owner) {
             return GetCodecBase(name, owner);

services/mediaresourcemanager/ResourceManagerService.cpp

diff --git a/services/mediaresourcemanager/ResourceManagerService.cpp b/services/mediaresourcemanager/ResourceManagerService.cpp
index e0584df..b4610bc 100644
--- a/services/mediaresourcemanager/ResourceManagerService.cpp
+++ b/services/mediaresourcemanager/ResourceManagerService.cpp
@@ -732,6 +732,7 @@
         return true;
     }
 
+    int failedClientPid = -1;
     {
         Mutex::Autolock lock(mLock);
         bool found = false;
@@ -746,11 +747,14 @@
                 }
             }
             if (found) {
+                failedClientPid = mMap.keyAt(i);
                 break;
             }
         }
-        if (!found) {
-            ALOGV("didn't find failed client");
+        if (found) {
+            ALOGW("Failed to reclaim resources from client with pid %d", failedClientPid);
+        } else {
+            ALOGW("Failed to reclaim resources from unlocateable client");
         }
     }