Fix issue #2163654: deadlock, runtime restart

Don't hold a lock when the activity thread is telling the activity manager
to release a provider.

This requires that the activity manager now keep a reference count on the
providers, because without the lock it is possible for activity thread to
call in to request the provider again before it has finished telling
about the release.

Change-Id: I5f912903891f4edae85e28819d4e6f14b8f2e688

3 files changed, 49 insertions, 22 deletions

diff --git a/core/java/android/app/ActivityThread.java b/core/java/android/app/ActivityThread.java
index 2cd223f..5f2de0f 100644
--- a/core/java/android/app/ActivityThread.java
+++ b/core/java/android/app/ActivityThread.java
@@ -4075,6 +4075,9 @@ public final class ActivityThread {
                 if(prc.count == 0) {
                     // Schedule the actual remove asynchronously, since we
                     // don't know the context this will be called in.
+                    // TODO: it would be nice to post a delayed message, so
+                    // if we come back and need the same provider quickly
+                    // we will still have it available.
                     Message msg = mH.obtainMessage(H.REMOVE_PROVIDER, provider);
                     mH.sendMessage(msg);
                 } //end if
@@ -4085,23 +4088,36 @@ public final class ActivityThread {
 
     final void completeRemoveProvider(IContentProvider provider) {
         IBinder jBinder = provider.asBinder();
+        String name = null;
         synchronized(mProviderMap) {
             ProviderRefCount prc = mProviderRefCountMap.get(jBinder);
             if(prc != null && prc.count == 0) {
                 mProviderRefCountMap.remove(jBinder);
                 //invoke removeProvider to dereference provider
-                removeProviderLocked(provider);
+                name = removeProviderLocked(provider);
             }
         }
+        
+        if (name != null) {
+            try {
+                if(localLOGV) Log.v(TAG, "removeProvider::Invoking " +
+                        "ActivityManagerNative.removeContentProvider(" + name);
+                ActivityManagerNative.getDefault().removeContentProvider(
+                        getApplicationThread(), name);
+            } catch (RemoteException e) {
+                //do nothing content provider object is dead any way
+            } //end catch
+        }
     }
     
-    public final void removeProviderLocked(IContentProvider provider) {
+    public final String removeProviderLocked(IContentProvider provider) {
         if (provider == null) {
-            return;
+            return null;
         }
         IBinder providerBinder = provider.asBinder();
-        boolean amRemoveFlag = false;
 
+        String name = null;
+        
         // remove the provider from mProviderMap
         Iterator<ProviderRecord> iter = mProviderMap.values().iterator();
         while (iter.hasNext()) {
@@ -4111,7 +4127,7 @@ public final class ActivityThread {
                 //find if its published by this process itself
                 if(pr.mLocalProvider != null) {
                     if(localLOGV) Log.i(TAG, "removeProvider::found local provider returning");
-                    return;
+                    return name;
                 }
                 if(localLOGV) Log.v(TAG, "removeProvider::Not local provider Unlinking " +
                         "death recipient");
@@ -4119,18 +4135,13 @@ public final class ActivityThread {
                 myBinder.unlinkToDeath(pr, 0);
                 iter.remove();
                 //invoke remove only once for the very first name seen
-                if(!amRemoveFlag) {
-                    try {
-                        if(localLOGV) Log.v(TAG, "removeProvider::Invoking " +
-                                "ActivityManagerNative.removeContentProvider("+pr.mName);
-                        ActivityManagerNative.getDefault().removeContentProvider(getApplicationThread(), pr.mName);
-                        amRemoveFlag = true;
-                    } catch (RemoteException e) {
-                        //do nothing content provider object is dead any way
-                    } //end catch
+                if(name == null) {
+                    name = pr.mName;
                 }
             } //end if myBinder
         }  //end while iter
+        
+        return name;
     }
 
     final void removeDeadProvider(String name, IContentProvider provider) {
diff --git a/services/java/com/android/server/am/ActivityManagerService.java b/services/java/com/android/server/am/ActivityManagerService.java
index 34302b1..c611795 100644
--- a/services/java/com/android/server/am/ActivityManagerService.java
+++ b/services/java/com/android/server/am/ActivityManagerService.java
@@ -7542,8 +7542,13 @@ public final class ActivityManagerService extends ActivityManagerNative implemen
                     if (DEBUG_PROVIDER) Log.v(TAG,
                             "Adding provider requested by "
                             + r.processName + " from process "
-                            + cpr.info.processName);                    
-                    r.conProviders.add(cpr);
+                            + cpr.info.processName);
+                    Integer cnt = r.conProviders.get(cpr);
+                    if (cnt == null) {
+                        r.conProviders.put(cpr, new Integer(1));
+                    } else {
+                        r.conProviders.put(cpr, new Integer(cnt.intValue()+1));
+                    }
                     cpr.clients.add(r);
                 } else {
                     cpr.externals++;
@@ -7648,8 +7653,13 @@ public final class ActivityManagerService extends ActivityManagerNative implemen
                     if (DEBUG_PROVIDER) Log.v(TAG,
                             "Adding provider requested by "
                             + r.processName + " from process "
-                            + cpr.info.processName);                    
-                    r.conProviders.add(cpr);
+                            + cpr.info.processName);
+                    Integer cnt = r.conProviders.get(cpr);
+                    if (cnt == null) {
+                        r.conProviders.put(cpr, new Integer(1));
+                    } else {
+                        r.conProviders.put(cpr, new Integer(cnt.intValue()+1));
+                    }
                     cpr.clients.add(r);
                 } else {
                     cpr.externals++;
@@ -7726,8 +7736,13 @@ public final class ActivityManagerService extends ActivityManagerNative implemen
                         + cpr.info.name + " in process " + r.processName);
                 return;
             } else {
-                localCpr.clients.remove(r);
-                r.conProviders.remove(localCpr);
+                Integer cnt = r.conProviders.get(localCpr);
+                if (cnt == null || cnt.intValue() <= 1) {
+                    localCpr.clients.remove(r);
+                    r.conProviders.remove(localCpr);
+                } else {
+                    r.conProviders.put(localCpr, new Integer(cnt.intValue()-1));
+                }
             }
             updateOomAdjLocked();
         }
@@ -9914,7 +9929,7 @@ public final class ActivityManagerService extends ActivityManagerNative implemen
 
         // Unregister from connected content providers.
         if (!app.conProviders.isEmpty()) {
-            Iterator it = app.conProviders.iterator();
+            Iterator it = app.conProviders.keySet().iterator();
             while (it.hasNext()) {
                 ContentProviderRecord cpr = (ContentProviderRecord)it.next();
                 cpr.clients.remove(app);
diff --git a/services/java/com/android/server/am/ProcessRecord.java b/services/java/com/android/server/am/ProcessRecord.java
index d05b44b..6202257 100644
--- a/services/java/com/android/server/am/ProcessRecord.java
+++ b/services/java/com/android/server/am/ProcessRecord.java
@@ -94,7 +94,8 @@ class ProcessRecord implements Watchdog.PssRequestor {
     // class (String) -> ContentProviderRecord
     final HashMap pubProviders = new HashMap(); 
     // All ContentProviderRecord process is using
-    final HashSet conProviders = new HashSet(); 
+    final HashMap<ContentProviderRecord, Integer> conProviders
+            = new HashMap<ContentProviderRecord, Integer>(); 
     
     boolean persistent;         // always keep this application running?
     boolean crashing;           // are we in the process of crashing?