From 5919ac6b4188285324646772501ef4b97b353cf4 Mon Sep 17 00:00:00 2001
From: Kenny Root <kroot@google.com>
Date: Tue, 5 Oct 2010 09:49:40 -0700
Subject: [PATCH] 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
---
 .../java/com/android/server/MountService.java | 58 ++++++++++++++-----
 1 file changed, 44 insertions(+), 14 deletions(-)

diff --git a/services/java/com/android/server/MountService.java b/services/java/com/android/server/MountService.java
index df42c5b019eff..06f9c41e707d2 100644
--- a/services/java/com/android/server/MountService.java
+++ b/services/java/com/android/server/MountService.java
@@ -170,8 +170,6 @@ class MountService extends IMountService.Stub
             this.token = token;
             this.callerUid = callerUid;
             mounted = false;
-
-            getBinder().linkToDeath(this, 0);
         }
 
         // OBB source filename
@@ -196,7 +194,11 @@ class MountService extends IMountService.Stub
             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 @@ class MountService extends IMountService.Stub
             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 @@ class MountService extends IMountService.Stub
 
     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 == null || obbStates.isEmpty()) {
-                mObbMounts.remove(obbState.getBinder());
-                obbState.cleanUp();
+                if (obbStates.remove(obbState)) {
+                    obbState.unlink();
+                }
+                if (obbStates.isEmpty()) {
+                    mObbMounts.remove(binder);
+                }
             }
+
             mObbPathToStateMap.remove(obbState.filename);
 
             // Track the number of OBBs used by this UID.
@@ -1708,7 +1738,7 @@ class MountService extends IMountService.Stub
         }
     }
 
-    private void replaceObbState(ObbState oldObbState, ObbState newObbState) {
+    private void replaceObbState(ObbState oldObbState, ObbState newObbState) throws RemoteException {
         synchronized (mObbMounts) {
             removeObbState(oldObbState);
             addObbState(newObbState);