commit e830527910ca2a78299e61ca817766e064651fa0
author Girish <girishshetty@google.com> Mon Dec 18 19:17:58 2023 +0000
committer Girish <girishshetty@google.com> Thu Jan 11 02:59:52 2024 +0000
tree 74aac25f1c08d657fbcefdfdbd69bc8d6fbe4091
parent d11a03a748207cdd7cb564fe02a9060a8d0ca97d

resourcemanager: protect resources from concurrent access

Since getClient access resources (map), make sure its protected
from the concurrent access.

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: Ica03c3dc45b97502414c76c10057acc9c9b0c03d

services/mediaresourcemanager/ResourceManagerService.cpp

diff --git a/services/mediaresourcemanager/ResourceManagerService.cpp b/services/mediaresourcemanager/ResourceManagerService.cpp
index 3a79fa0..3a02443 100644
--- a/services/mediaresourcemanager/ResourceManagerService.cpp
+++ b/services/mediaresourcemanager/ResourceManagerService.cpp
@@ -620,7 +620,7 @@
     mResourceManagerMetrics->pushReclaimAtom(clientInfo, priorities, targetClients, reclaimed);
 }
 
-std::shared_ptr<IResourceManagerClient> ResourceManagerService::getClient(
+std::shared_ptr<IResourceManagerClient> ResourceManagerService::getClient_l(
         int pid, const int64_t& clientId) const {
     std::map<int, ResourceInfos>::const_iterator found = mMap.find(pid);
     if (found == mMap.end()) {
@@ -638,7 +638,7 @@
     return foundClient->second.client;
 }
 
-bool ResourceManagerService::removeClient(int pid, const int64_t& clientId) {
+bool ResourceManagerService::removeClient_l(int pid, const int64_t& clientId) {
     std::map<int, ResourceInfos>::iterator found = mMap.find(pid);
     if (found == mMap.end()) {
         ALOGV("%s: didn't find pid %d for clientId %lld", __func__, pid, (long long) clientId);
@@ -665,8 +665,11 @@
     int64_t failedClientId = -1;
     int32_t failedClientPid = -1;
     for (const ClientInfo& targetClient : targetClients) {
-        std::shared_ptr<IResourceManagerClient> client = getClient(
-            targetClient.mPid, targetClient.mClientId);
+        std::shared_ptr<IResourceManagerClient> client = nullptr;
+        {
+            std::scoped_lock lock{mLock};
+            client = getClient_l(targetClient.mPid, targetClient.mClientId);
+        }
         if (client == nullptr) {
             // skip already released clients.
             continue;
@@ -688,7 +691,7 @@
 
     {
         std::scoped_lock lock{mLock};
-        bool found = removeClient(failedClientPid, failedClientId);
+        bool found = removeClient_l(failedClientPid, failedClientId);
         if (found) {
             ALOGW("Failed to reclaim resources from client with pid %d", failedClientPid);
         } else {