From b6c79ea065fc1adf383beabfacb401ff8b6466a8 Mon Sep 17 00:00:00 2001
From: Makoto Onuki <omakoto@google.com>
Date: Thu, 31 May 2018 11:03:56 -0700
Subject: [PATCH] Serialize AM.getContentProvider() calls in client side

Th wait() call in AMS.getContentProviderImpl() can cause a deadlock when it
starts a new process for a target provider and all system server binder threads
are reaching out to the same provider at the same time, because AMS won't be
able to receive a callback from the provider process.

Let's serialize access to AM.getContentProvider() to avoid this scenario.

Longer team, we should stop waiting in the system server and instead do so in
the client side.

Bug: 74523247
Test: Boot, add a google account, and let syncs run and finish.
Test: Watch free video on Coursera

Change-Id: If5be2dc4b6f22f72d1bb3aa1b5b4d49f20c8d94d
---
 core/java/android/app/ActivityThread.java | 25 +++++++++++++++++++++--
 1 file changed, 23 insertions(+), 2 deletions(-)

diff --git a/core/java/android/app/ActivityThread.java b/core/java/android/app/ActivityThread.java
index 37bf21a898b14..87b334992d2da 100644
--- a/core/java/android/app/ActivityThread.java
+++ b/core/java/android/app/ActivityThread.java
@@ -347,6 +347,13 @@ public final class ActivityThread extends ClientTransactionHandler {
     final ArrayMap<ComponentName, ProviderClientRecord> mLocalProvidersByName
             = new ArrayMap<ComponentName, ProviderClientRecord>();
 
+    // Mitigation for b/74523247: Used to serialize calls to AM.getContentProvider().
+    // Note we never removes items from this map but that's okay because there are only so many
+    // users and so many authorities.
+    // TODO Remove it once we move CPR.wait() from AMS to the client side.
+    @GuardedBy("mGetProviderLocks")
+    final ArrayMap<ProviderKey, Object> mGetProviderLocks = new ArrayMap<>();
+
     final ArrayMap<Activity, ArrayList<OnActivityPausedListener>> mOnPauseListeners
         = new ArrayMap<Activity, ArrayList<OnActivityPausedListener>>();
 
@@ -5959,8 +5966,10 @@ public final class ActivityThread extends ClientTransactionHandler {
         // be re-entrant in the case where the provider is in the same process.
         ContentProviderHolder holder = null;
         try {
-            holder = ActivityManager.getService().getContentProvider(
-                    getApplicationThread(), auth, userId, stable);
+            synchronized (getGetProviderLock(auth, userId)) {
+                holder = ActivityManager.getService().getContentProvider(
+                        getApplicationThread(), auth, userId, stable);
+            }
         } catch (RemoteException ex) {
             throw ex.rethrowFromSystemServer();
         }
@@ -5976,6 +5985,18 @@ public final class ActivityThread extends ClientTransactionHandler {
         return holder.provider;
     }
 
+    private Object getGetProviderLock(String auth, int userId) {
+        final ProviderKey key = new ProviderKey(auth, userId);
+        synchronized (mGetProviderLocks) {
+            Object lock = mGetProviderLocks.get(key);
+            if (lock == null) {
+                lock = key;
+                mGetProviderLocks.put(key, lock);
+            }
+            return lock;
+        }
+    }
+
     private final void incProviderRefLocked(ProviderRefCount prc, boolean stable) {
         if (stable) {
             prc.stableCount += 1;