From 6fe30c4440a9702adec10e7fcc70c1e10f97c484 Mon Sep 17 00:00:00 2001
From: Shashwat Razdan <srazdan@google.com>
Date: Wed, 9 Oct 2024 14:03:37 -0700
Subject: Add per-package lock in setAppFunctionEnabled.

Bug: 357551503
Test: CTS
Flag: android.app.appfunctions.flags.enable_app_function_manager
Change-Id: I3c46a6ef1cdb07e6e1a30a9d1417fac8c55574f3
---
 .../AppFunctionManagerServiceImpl.java             | 34 +++++++++++++++++-----
 1 file changed, 27 insertions(+), 7 deletions(-)

(limited to 'services/appfunctions/java')

diff --git a/services/appfunctions/java/com/android/server/appfunctions/AppFunctionManagerServiceImpl.java b/services/appfunctions/java/com/android/server/appfunctions/AppFunctionManagerServiceImpl.java
index c5fef191c52c..2ee7561bd631 100644
--- a/services/appfunctions/java/com/android/server/appfunctions/AppFunctionManagerServiceImpl.java
+++ b/services/appfunctions/java/com/android/server/appfunctions/AppFunctionManagerServiceImpl.java
@@ -65,12 +65,13 @@ import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.infra.AndroidFuture;
 import com.android.internal.util.DumpUtils;
 import com.android.server.SystemService.TargetUser;
-import com.android.server.appfunctions.RemoteServiceCaller.RunServiceCallCallback;
-import com.android.server.appfunctions.RemoteServiceCaller.ServiceUsageCompleteListener;
 
 import java.io.FileDescriptor;
 import java.io.PrintWriter;
+import java.util.Collections;
+import java.util.Map;
 import java.util.Objects;
+import java.util.WeakHashMap;
 import java.util.concurrent.CompletionException;
 import java.util.concurrent.Executor;
 
@@ -83,7 +84,8 @@ public class AppFunctionManagerServiceImpl extends IAppFunctionManager.Stub {
     private final ServiceHelper mInternalServiceHelper;
     private final ServiceConfig mServiceConfig;
     private final Context mContext;
-    private final Object mLock = new Object();
+    private final Map<String, Object> mLocks = new WeakHashMap<>();
+
 
     public AppFunctionManagerServiceImpl(@NonNull Context context) {
         this(
@@ -321,9 +323,7 @@ public class AppFunctionManagerServiceImpl extends IAppFunctionManager.Stub {
         THREAD_POOL_EXECUTOR.execute(
                 () -> {
                     try {
-                        // TODO(357551503): Instead of holding a global lock, hold a per-package
-                        //  lock.
-                        synchronized (mLock) {
+                        synchronized (getLockForPackage(callingPackage)) {
                             setAppFunctionEnabledInternalLocked(
                                     callingPackage, functionIdentifier, userHandle, enabledState);
                         }
@@ -351,7 +351,7 @@ public class AppFunctionManagerServiceImpl extends IAppFunctionManager.Stub {
      * process.
      */
     @WorkerThread
-    @GuardedBy("mLock")
+    @GuardedBy("getLockForPackage(callingPackage)")
     private void setAppFunctionEnabledInternalLocked(
             @NonNull String callingPackage,
             @NonNull String functionIdentifier,
@@ -545,6 +545,26 @@ public class AppFunctionManagerServiceImpl extends IAppFunctionManager.Stub {
                                     });
         }
     }
+    /**
+     * Retrieves the lock object associated with the given package name.
+     *
+     * This method returns the lock object from the {@code mLocks} map if it exists.
+     * If no lock is found for the given package name, a new lock object is created,
+     * stored in the map, and returned.
+     */
+    @VisibleForTesting
+    @NonNull
+    Object getLockForPackage(String callingPackage) {
+        // Synchronized the access to mLocks to prevent race condition.
+        synchronized (mLocks) {
+            // By using a WeakHashMap, we allow the garbage collector to reclaim memory by removing
+            // entries associated with unused callingPackage keys. Therefore, we remove the null
+            // values before getting/computing a new value. The goal is to not let the size of this
+            // map grow without an upper bound.
+            mLocks.values().removeAll(Collections.singleton(null)); // Remove null values
+            return mLocks.computeIfAbsent(callingPackage, k -> new Object());
+        }
+    }
 
     private static class AppFunctionMetadataObserver implements ObserverCallback {
         @Nullable private final MetadataSyncAdapter mPerUserMetadataSyncAdapter;
-- 
cgit v1.2.3-59-g8ed1b