Merge "Fix races when content providers are acquired and released." into ics-mr1
diff --git a/core/java/android/app/ActivityThread.java b/core/java/android/app/ActivityThread.java
index f9896f7..303f81b 100644
--- a/core/java/android/app/ActivityThread.java
+++ b/core/java/android/app/ActivityThread.java
@@ -2734,8 +2734,9 @@
         CharSequence description;
     }
 
-    private class ProviderRefCount {
+    private static final class ProviderRefCount {
         public int count;
+
         ProviderRefCount(int pCount) {
             count = pCount;
         }
@@ -3988,16 +3989,14 @@
             buf.append(": ");
             buf.append(cpi.name);
             Log.i(TAG, buf.toString());
-            IContentProvider cp = installProvider(context, null, cpi, false);
+            IContentProvider cp = installProvider(context, null, cpi,
+                    false /*noisy*/, true /*noReleaseNeeded*/);
             if (cp != null) {
                 IActivityManager.ContentProviderHolder cph =
-                    new IActivityManager.ContentProviderHolder(cpi);
+                        new IActivityManager.ContentProviderHolder(cpi);
                 cph.provider = cp;
+                cph.noReleaseNeeded = true;
                 results.add(cph);
-                // Don't ever unload this provider from the process.
-                synchronized(mProviderMap) {
-                    mProviderRefCountMap.put(cp.asBinder(), new ProviderRefCount(10000));
-                }
             }
         }
 
@@ -4008,26 +4007,22 @@
         }
     }
 
-    private IContentProvider getExistingProvider(Context context, String name) {
-        synchronized(mProviderMap) {
-            final ProviderClientRecord pr = mProviderMap.get(name);
-            if (pr != null) {
-                return pr.mProvider;
-            }
-            return null;
-        }
-    }
-
-    private IContentProvider getProvider(Context context, String name) {
-        IContentProvider existing = getExistingProvider(context, name);
-        if (existing != null) {
-            return existing;
+    public final IContentProvider acquireProvider(Context c, String name) {
+        IContentProvider provider = acquireExistingProvider(c, name);
+        if (provider != null) {
+            return provider;
         }
 
+        // There is a possible race here.  Another thread may try to acquire
+        // the same provider at the same time.  When this happens, we want to ensure
+        // that the first one wins.
+        // Note that we cannot hold the lock while acquiring and installing the
+        // provider since it might take a long time to run and it could also potentially
+        // be re-entrant in the case where the provider is in the same process.
         IActivityManager.ContentProviderHolder holder = null;
         try {
             holder = ActivityManagerNative.getDefault().getContentProvider(
-                getApplicationThread(), name);
+                    getApplicationThread(), name);
         } catch (RemoteException ex) {
         }
         if (holder == null) {
@@ -4035,136 +4030,137 @@
             return null;
         }
 
-        IContentProvider prov = installProvider(context, holder.provider,
-                holder.info, true);
-        //Slog.i(TAG, "noReleaseNeeded=" + holder.noReleaseNeeded);
-        if (holder.noReleaseNeeded || holder.provider == null) {
-            // We are not going to release the provider if it is an external
-            // provider that doesn't care about being released, or if it is
-            // a local provider running in this process.
-            //Slog.i(TAG, "*** NO RELEASE NEEDED");
-            synchronized(mProviderMap) {
-                mProviderRefCountMap.put(prov.asBinder(), new ProviderRefCount(10000));
+        // Install provider will increment the reference count for us, and break
+        // any ties in the race.
+        provider = installProvider(c, holder.provider, holder.info,
+                true /*noisy*/, holder.noReleaseNeeded);
+        if (holder.provider != null && provider != holder.provider) {
+            if (localLOGV) {
+                Slog.v(TAG, "acquireProvider: lost the race, releasing extraneous "
+                        + "reference to the content provider");
+            }
+            try {
+                ActivityManagerNative.getDefault().removeContentProvider(
+                        getApplicationThread(), name);
+            } catch (RemoteException ex) {
             }
         }
-        return prov;
-    }
-
-    public final IContentProvider acquireProvider(Context c, String name) {
-        IContentProvider provider = getProvider(c, name);
-        if(provider == null)
-            return null;
-        IBinder jBinder = provider.asBinder();
-        synchronized(mProviderMap) {
-            ProviderRefCount prc = mProviderRefCountMap.get(jBinder);
-            if(prc == null) {
-                mProviderRefCountMap.put(jBinder, new ProviderRefCount(1));
-            } else {
-                prc.count++;
-            } //end else
-        } //end synchronized
         return provider;
     }
 
     public final IContentProvider acquireExistingProvider(Context c, String name) {
-        IContentProvider provider = getExistingProvider(c, name);
-        if(provider == null)
-            return null;
-        IBinder jBinder = provider.asBinder();
-        synchronized(mProviderMap) {
+        synchronized (mProviderMap) {
+            ProviderClientRecord pr = mProviderMap.get(name);
+            if (pr == null) {
+                return null;
+            }
+
+            IContentProvider provider = pr.mProvider;
+            IBinder jBinder = provider.asBinder();
+
+            // Only increment the ref count if we have one.  If we don't then the
+            // provider is not reference counted and never needs to be released.
             ProviderRefCount prc = mProviderRefCountMap.get(jBinder);
-            if(prc == null) {
-                mProviderRefCountMap.put(jBinder, new ProviderRefCount(1));
-            } else {
-                prc.count++;
-            } //end else
-        } //end synchronized
-        return provider;
+            if (prc != null) {
+                prc.count += 1;
+                if (prc.count == 1) {
+                    if (localLOGV) {
+                        Slog.v(TAG, "acquireExistingProvider: "
+                                + "snatched provider from the jaws of death");
+                    }
+                    // Because the provider previously had a reference count of zero,
+                    // it was scheduled to be removed.  Cancel that.
+                    mH.removeMessages(H.REMOVE_PROVIDER, provider);
+                }
+            }
+            return provider;
+        }
     }
 
     public final boolean releaseProvider(IContentProvider provider) {
         if(provider == null) {
             return false;
         }
+
         IBinder jBinder = provider.asBinder();
-        synchronized(mProviderMap) {
+        synchronized (mProviderMap) {
             ProviderRefCount prc = mProviderRefCountMap.get(jBinder);
-            if(prc == null) {
-                if(localLOGV) Slog.v(TAG, "releaseProvider::Weird shouldn't be here");
+            if (prc == null) {
+                // The provider has no ref count, no release is needed.
                 return false;
-            } else {
-                prc.count--;
-                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
-            } //end else
-        } //end synchronized
-        return true;
+            }
+
+            if (prc.count == 0) {
+                if (localLOGV) Slog.v(TAG, "releaseProvider: ref count already 0, how?");
+                return false;
+            }
+
+            prc.count -= 1;
+            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);
+            }
+            return true;
+        }
     }
 
     final void completeRemoveProvider(IContentProvider provider) {
         IBinder jBinder = provider.asBinder();
-        String name = null;
+        String remoteProviderName = null;
         synchronized(mProviderMap) {
             ProviderRefCount prc = mProviderRefCountMap.get(jBinder);
-            if(prc != null && prc.count == 0) {
-                mProviderRefCountMap.remove(jBinder);
-                //invoke removeProvider to dereference provider
-                name = removeProviderLocked(provider);
+            if (prc == null) {
+                // Either no release is needed (so we shouldn't be here) or the
+                // provider was already released.
+                if (localLOGV) Slog.v(TAG, "completeRemoveProvider: release not needed");
+                return;
+            }
+
+            if (prc.count != 0) {
+                // There was a race!  Some other client managed to acquire
+                // the provider before the removal was completed.
+                // Abort the removal.  We will do it later.
+                if (localLOGV) Slog.v(TAG, "completeRemoveProvider: lost the race, "
+                        + "provider still in use");
+                return;
+            }
+
+            mProviderRefCountMap.remove(jBinder);
+
+            Iterator<ProviderClientRecord> iter = mProviderMap.values().iterator();
+            while (iter.hasNext()) {
+                ProviderClientRecord pr = iter.next();
+                IBinder myBinder = pr.mProvider.asBinder();
+                if (myBinder == jBinder) {
+                    iter.remove();
+                    if (pr.mLocalProvider == null) {
+                        myBinder.unlinkToDeath(pr, 0);
+                        if (remoteProviderName == null) {
+                            remoteProviderName = pr.mName;
+                        }
+                    }
+                }
             }
         }
-        
-        if (name != null) {
+
+        if (remoteProviderName != null) {
             try {
-                if(localLOGV) Slog.v(TAG, "removeProvider::Invoking " +
-                        "ActivityManagerNative.removeContentProvider(" + name);
+                if (localLOGV) {
+                    Slog.v(TAG, "removeProvider: Invoking ActivityManagerNative."
+                            + "removeContentProvider(" + remoteProviderName + ")");
+                }
                 ActivityManagerNative.getDefault().removeContentProvider(
-                        getApplicationThread(), name);
+                        getApplicationThread(), remoteProviderName);
             } catch (RemoteException e) {
                 //do nothing content provider object is dead any way
-            } //end catch
+            }
         }
     }
-    
-    public final String removeProviderLocked(IContentProvider provider) {
-        if (provider == null) {
-            return null;
-        }
-        IBinder providerBinder = provider.asBinder();
-
-        String name = null;
-        
-        // remove the provider from mProviderMap
-        Iterator<ProviderClientRecord> iter = mProviderMap.values().iterator();
-        while (iter.hasNext()) {
-            ProviderClientRecord pr = iter.next();
-            IBinder myBinder = pr.mProvider.asBinder();
-            if (myBinder == providerBinder) {
-                //find if its published by this process itself
-                if(pr.mLocalProvider != null) {
-                    if(localLOGV) Slog.i(TAG, "removeProvider::found local provider returning");
-                    return name;
-                }
-                if(localLOGV) Slog.v(TAG, "removeProvider::Not local provider Unlinking " +
-                        "death recipient");
-                //content provider is in another process
-                myBinder.unlinkToDeath(pr, 0);
-                iter.remove();
-                //invoke remove only once for the very first name seen
-                if(name == null) {
-                    name = pr.mName;
-                }
-            } //end if myBinder
-        }  //end while iter
-        
-        return name;
-    }
 
     final void removeDeadProvider(String name, IContentProvider provider) {
         synchronized(mProviderMap) {
@@ -4179,8 +4175,23 @@
         }
     }
 
+    /**
+     * Installs the provider.
+     *
+     * Providers that are local to the process or that come from the system server
+     * may be installed permanently which is indicated by setting noReleaseNeeded to true.
+     * Other remote providers are reference counted.  The initial reference count
+     * for all reference counted providers is one.  Providers that are not reference
+     * counted do not have a reference count (at all).
+     *
+     * This method detects when a provider has already been installed.  When this happens,
+     * it increments the reference count of the existing provider (if appropriate)
+     * and returns the existing provider.  This can happen due to concurrent
+     * attempts to acquire the same provider.
+     */
     private IContentProvider installProvider(Context context,
-            IContentProvider provider, ProviderInfo info, boolean noisy) {
+            IContentProvider provider, ProviderInfo info,
+            boolean noisy, boolean noReleaseNeeded) {
         ContentProvider localProvider = null;
         if (provider == null) {
             if (noisy) {
@@ -4238,24 +4249,69 @@
         }
 
         synchronized (mProviderMap) {
-            // Cache the pointer for the remote provider.
+            // There is a possibility that this thread raced with another thread to
+            // add the provider.  If we find another thread got there first then we
+            // just get out of the way and return the original provider.
+            IBinder jBinder = provider.asBinder();
             String names[] = PATTERN_SEMICOLON.split(info.authority);
-            for (int i=0; i<names.length; i++) {
-                ProviderClientRecord pr = new ProviderClientRecord(names[i], provider,
-                        localProvider);
-                try {
-                    provider.asBinder().linkToDeath(pr, 0);
+            for (int i = 0; i < names.length; i++) {
+                ProviderClientRecord pr = mProviderMap.get(names[i]);
+                if (pr != null) {
+                    if (localLOGV) {
+                        Slog.v(TAG, "installProvider: lost the race, "
+                                + "using existing named provider");
+                    }
+                    provider = pr.mProvider;
+                } else {
+                    pr = new ProviderClientRecord(names[i], provider, localProvider);
+                    if (localProvider == null) {
+                        try {
+                            jBinder.linkToDeath(pr, 0);
+                        } catch (RemoteException e) {
+                            // Provider already dead.  Bail out of here without making
+                            // any changes to the provider map or other data structures.
+                            return null;
+                        }
+                    }
                     mProviderMap.put(names[i], pr);
-                } catch (RemoteException e) {
-                    return null;
                 }
             }
+
             if (localProvider != null) {
-                mLocalProviders.put(provider.asBinder(),
-                        new ProviderClientRecord(null, provider, localProvider));
+                ProviderClientRecord pr = mLocalProviders.get(jBinder);
+                if (pr != null) {
+                    if (localLOGV) {
+                        Slog.v(TAG, "installProvider: lost the race, "
+                                + "using existing local provider");
+                    }
+                    provider = pr.mProvider;
+                } else {
+                    pr = new ProviderClientRecord(null, provider, localProvider);
+                    mLocalProviders.put(jBinder, pr);
+                }
+            }
+
+            if (!noReleaseNeeded) {
+                ProviderRefCount prc = mProviderRefCountMap.get(jBinder);
+                if (prc != null) {
+                    if (localLOGV) {
+                        Slog.v(TAG, "installProvider: lost the race, incrementing ref count");
+                    }
+                    prc.count += 1;
+                    if (prc.count == 1) {
+                        if (localLOGV) {
+                            Slog.v(TAG, "installProvider: "
+                                    + "snatched provider from the jaws of death");
+                        }
+                        // Because the provider previously had a reference count of zero,
+                        // it was scheduled to be removed.  Cancel that.
+                        mH.removeMessages(H.REMOVE_PROVIDER, provider);
+                    }
+                } else {
+                    mProviderRefCountMap.put(jBinder, new ProviderRefCount(1));
+                }
             }
         }
-
         return provider;
     }