commit 5919ac6b4188285324646772501ef4b97b353cf4
author Kenny Root <kroot@google.com> Tue Oct 05 09:49:40 2010 -0700
committer Kenny Root <kroot@google.com> Wed Oct 06 17:00:00 2010 -0700
tree 24fd16d8c43c44500f9cf39d7fa0f93cd0e88546
parent a3ee5c0351e51f7a978ddddc37644f46051ca6ed

OBB: track binder death observers

An incorrect assumption about how death observers were tracked lead to
an IllegalArgumentException in some cases. Make sure the linking and
unlinking of the Binder to its ObbState death observer is symmetric to
avoid this problem.

Bug: 3062360
Change-Id: Idd016db12551c80cd74d00f11cf6569bd3b4ce21

services/java/com/android/server/MountService.java

diff --git a/services/java/com/android/server/MountService.java b/services/java/com/android/server/MountService.java
index df42c5b..06f9c41 100644
--- a/services/java/com/android/server/MountService.java
+++ b/services/java/com/android/server/MountService.java
@@ -170,8 +170,6 @@
             this.token = token;
             this.callerUid = callerUid;
             mounted = false;
-
-            getBinder().linkToDeath(this, 0);
         }
 
         // OBB source filename
@@ -196,7 +194,11 @@
             mObbActionHandler.sendMessage(mObbActionHandler.obtainMessage(OBB_RUN_ACTION, action));
         }
 
-        public void cleanUp() {
+        public void link() throws RemoteException {
+            getBinder().linkToDeath(this, 0);
+        }
+
+        public void unlink() {
             getBinder().unlinkToDeath(this, 0);
         }
 
@@ -1659,14 +1661,39 @@
             Slog.i(TAG, "Send to OBB handler: " + action.toString());
     }
 
-    private void addObbState(ObbState obbState) {
+    private void addObbState(ObbState obbState) throws RemoteException {
         synchronized (mObbMounts) {
-            List<ObbState> obbStates = mObbMounts.get(obbState.getBinder());
+            final IBinder binder = obbState.getBinder();
+            List<ObbState> obbStates = mObbMounts.get(binder);
+            final boolean unique;
+
             if (obbStates == null) {
                 obbStates = new ArrayList<ObbState>();
-                mObbMounts.put(obbState.getBinder(), obbStates);
+                mObbMounts.put(binder, obbStates);
+                unique = true;
+            } else {
+                unique = obbStates.contains(obbState);
             }
-            obbStates.add(obbState);
+
+            if (unique) {
+                obbStates.add(obbState);
+                try {
+                    obbState.link();
+                } catch (RemoteException e) {
+                    /*
+                     * The binder died before we could link it, so clean up our
+                     * state and return failure.
+                     */
+                    obbStates.remove(obbState);
+                    if (obbStates.isEmpty()) {
+                        mObbMounts.remove(binder);
+                    }
+
+                    // Rethrow the error so mountObb can get it
+                    throw e;
+                }
+            }
+
             mObbPathToStateMap.put(obbState.filename, obbState);
 
             // Track the number of OBBs used by this UID.
@@ -1682,14 +1709,17 @@
 
     private void removeObbState(ObbState obbState) {
         synchronized (mObbMounts) {
-            final List<ObbState> obbStates = mObbMounts.get(obbState.getBinder());
+            final IBinder binder = obbState.getBinder();
+            final List<ObbState> obbStates = mObbMounts.get(binder);
             if (obbStates != null) {
-                obbStates.remove(obbState);
+                if (obbStates.remove(obbState)) {
+                    obbState.unlink();
+                }
+                if (obbStates.isEmpty()) {
+                    mObbMounts.remove(binder);
+                }
             }
-            if (obbStates == null || obbStates.isEmpty()) {
-                mObbMounts.remove(obbState.getBinder());
-                obbState.cleanUp();
-            }
+
             mObbPathToStateMap.remove(obbState.filename);
 
             // Track the number of OBBs used by this UID.
@@ -1708,7 +1738,7 @@
         }
     }
 
-    private void replaceObbState(ObbState oldObbState, ObbState newObbState) {
+    private void replaceObbState(ObbState oldObbState, ObbState newObbState) throws RemoteException {
         synchronized (mObbMounts) {
             removeObbState(oldObbState);
             addObbState(newObbState);