Make it safe to use start/stop app ops outside of system proc

We now keep track of all of the active start operations per
non-system process, so they can be cleaned up if the process
goes away.

Change-Id: I9d05f1e0281c47dbe1213de014f0491f1359685c

6 files changed, 191 insertions, 39 deletions

diff --git a/core/java/android/app/AppOpsManager.java b/core/java/android/app/AppOpsManager.java
index c22de0c..8d47236 100644
--- a/core/java/android/app/AppOpsManager.java
+++ b/core/java/android/app/AppOpsManager.java
@@ -16,6 +16,8 @@
 
 package android.app;
 
+import android.os.Binder;
+import android.os.IBinder;
 import android.util.ArrayMap;
 import com.android.internal.app.IAppOpsService;
 import com.android.internal.app.IAppOpsCallback;
@@ -55,6 +57,8 @@ public class AppOpsManager {
     final ArrayMap<Callback, IAppOpsCallback> mModeWatchers
             = new ArrayMap<Callback, IAppOpsCallback>();
 
+    static IBinder sToken;
+
     public static final int MODE_ALLOWED = 0;
     public static final int MODE_IGNORED = 1;
     public static final int MODE_ERRORED = 2;
@@ -640,6 +644,21 @@ public class AppOpsManager {
         return noteOp(op, Process.myUid(), mContext.getBasePackageName());
     }
 
+    /** @hide */
+    public static IBinder getToken(IAppOpsService service) {
+        synchronized (AppOpsManager.class) {
+            if (sToken != null) {
+                return sToken;
+            }
+            try {
+                sToken = service.getToken(new Binder());
+            } catch (RemoteException e) {
+                // System is dead, whatevs.
+            }
+            return sToken;
+        }
+    }
+
     /**
      * Report that an application has started executing a long-running operation.  Note that you
      * must pass in both the uid and name of the application to be checked; this function will
@@ -658,7 +677,7 @@ public class AppOpsManager {
      */
     public int startOp(int op, int uid, String packageName) {
         try {
-            int mode = mService.startOperation(op, uid, packageName);
+            int mode = mService.startOperation(getToken(mService), op, uid, packageName);
             if (mode == MODE_ERRORED) {
                 throw new SecurityException("Operation not allowed");
             }
@@ -674,7 +693,7 @@ public class AppOpsManager {
      */
     public int startOpNoThrow(int op, int uid, String packageName) {
         try {
-            return mService.startOperation(op, uid, packageName);
+            return mService.startOperation(getToken(mService), op, uid, packageName);
         } catch (RemoteException e) {
         }
         return MODE_IGNORED;
@@ -693,7 +712,7 @@ public class AppOpsManager {
      */
     public void finishOp(int op, int uid, String packageName) {
         try {
-            mService.finishOperation(op, uid, packageName);
+            mService.finishOperation(getToken(mService), op, uid, packageName);
         } catch (RemoteException e) {
         }
     }
diff --git a/core/java/com/android/internal/app/IAppOpsService.aidl b/core/java/com/android/internal/app/IAppOpsService.aidl
index a9da863..2dc822e 100644
--- a/core/java/com/android/internal/app/IAppOpsService.aidl
+++ b/core/java/com/android/internal/app/IAppOpsService.aidl
@@ -24,10 +24,11 @@ interface IAppOpsService {
     // be kept in sync with frameworks/native/include/binder/IAppOpsService.h
     int checkOperation(int code, int uid, String packageName);
     int noteOperation(int code, int uid, String packageName);
-    int startOperation(int code, int uid, String packageName);
-    void finishOperation(int code, int uid, String packageName);
+    int startOperation(IBinder token, int code, int uid, String packageName);
+    void finishOperation(IBinder token, int code, int uid, String packageName);
     void startWatchingMode(int op, String packageName, IAppOpsCallback callback);
     void stopWatchingMode(IAppOpsCallback callback);
+    IBinder getToken(IBinder clientToken);
 
     // Remaining methods are only used in Java.
     List<AppOpsManager.PackageOps> getPackagesForOps(in int[] ops);
diff --git a/services/java/com/android/server/AppOpsService.java b/services/java/com/android/server/AppOpsService.java
index 7a107e7..6b4d248 100644
--- a/services/java/com/android/server/AppOpsService.java
+++ b/services/java/com/android/server/AppOpsService.java
@@ -40,6 +40,7 @@ import android.os.Process;
 import android.os.RemoteException;
 import android.os.ServiceManager;
 import android.os.UserHandle;
+import android.util.ArrayMap;
 import android.util.AtomicFile;
 import android.util.Log;
 import android.util.Slog;
@@ -97,6 +98,8 @@ public class AppOpsService extends IAppOpsService.Stub {
     }
 
     public final static class Op {
+        public final int uid;
+        public final String packageName;
         public final int op;
         public int mode;
         public int duration;
@@ -104,7 +107,9 @@ public class AppOpsService extends IAppOpsService.Stub {
         public long rejectTime;
         public int nesting;
 
-        public Op(int _op) {
+        public Op(int _uid, String _packageName, int _op) {
+            uid = _uid;
+            packageName = _packageName;
             op = _op;
             mode = AppOpsManager.MODE_ALLOWED;
         }
@@ -112,10 +117,10 @@ public class AppOpsService extends IAppOpsService.Stub {
 
     final SparseArray<ArrayList<Callback>> mOpModeWatchers
             = new SparseArray<ArrayList<Callback>>();
-    final HashMap<String, ArrayList<Callback>> mPackageModeWatchers
-            = new HashMap<String, ArrayList<Callback>>();
-    final HashMap<IBinder, Callback> mModeWatchers
-            = new HashMap<IBinder, Callback>();
+    final ArrayMap<String, ArrayList<Callback>> mPackageModeWatchers
+            = new ArrayMap<String, ArrayList<Callback>>();
+    final ArrayMap<IBinder, Callback> mModeWatchers
+            = new ArrayMap<IBinder, Callback>();
 
     public final class Callback implements DeathRecipient {
         final IAppOpsCallback mCallback;
@@ -138,6 +143,47 @@ public class AppOpsService extends IAppOpsService.Stub {
         }
     }
 
+    final ArrayMap<IBinder, ClientState> mClients = new ArrayMap<IBinder, ClientState>();
+
+    public final class ClientState extends Binder implements DeathRecipient {
+        final IBinder mAppToken;
+        final int mPid;
+        final ArrayList<Op> mStartedOps;
+
+        public ClientState(IBinder appToken) {
+            mAppToken = appToken;
+            mPid = Binder.getCallingPid();
+            if (appToken instanceof Binder) {
+                // For local clients, there is no reason to track them.
+                mStartedOps = null;
+            } else {
+                mStartedOps = new ArrayList<Op>();
+                try {
+                    mAppToken.linkToDeath(this, 0);
+                } catch (RemoteException e) {
+                }
+            }
+        }
+
+        @Override
+        public String toString() {
+            return "ClientState{" +
+                    "mAppToken=" + mAppToken +
+                    ", " + (mStartedOps != null ? ("pid=" + mPid) : "local") +
+                    '}';
+        }
+
+        @Override
+        public void binderDied() {
+            synchronized (AppOpsService.this) {
+                for (int i=mStartedOps.size()-1; i>=0; i--) {
+                    finishOperationLocked(mStartedOps.get(i));
+                }
+                mClients.remove(mAppToken);
+            }
+        }
+    }
+
     public AppOpsService(File storagePath) {
         mFile = new AtomicFile(storagePath);
         mHandler = new Handler();
@@ -380,21 +426,18 @@ public class AppOpsService extends IAppOpsService.Stub {
             Callback cb = mModeWatchers.remove(callback.asBinder());
             if (cb != null) {
                 cb.unlinkToDeath();
-                for (int i=0; i<mOpModeWatchers.size(); i++) {
+                for (int i=mOpModeWatchers.size()-1; i>=0; i--) {
                     ArrayList<Callback> cbs = mOpModeWatchers.valueAt(i);
                     cbs.remove(cb);
                     if (cbs.size() <= 0) {
                         mOpModeWatchers.removeAt(i);
                     }
                 }
-                if (mPackageModeWatchers.size() > 0) {
-                    Iterator<ArrayList<Callback>> it = mPackageModeWatchers.values().iterator();
-                    while (it.hasNext()) {
-                        ArrayList<Callback> cbs = it.next();
-                        cbs.remove(cb);
-                        if (cbs.size() <= 0) {
-                            it.remove();
-                        }
+                for (int i=mPackageModeWatchers.size()-1; i>=0; i--) {
+                    ArrayList<Callback> cbs = mPackageModeWatchers.valueAt(i);
+                    cbs.remove(cb);
+                    if (cbs.size() <= 0) {
+                        mPackageModeWatchers.removeAt(i);
                     }
                 }
             }
@@ -402,6 +445,18 @@ public class AppOpsService extends IAppOpsService.Stub {
     }
 
     @Override
+    public IBinder getToken(IBinder clientToken) {
+        synchronized (this) {
+            ClientState cs = mClients.get(clientToken);
+            if (cs == null) {
+                cs = new ClientState(clientToken);
+                mClients.put(clientToken, cs);
+            }
+            return cs;
+        }
+    }
+
+    @Override
     public int checkOperation(int code, int uid, String packageName) {
         verifyIncomingUid(uid);
         verifyIncomingOp(code);
@@ -448,9 +503,10 @@ public class AppOpsService extends IAppOpsService.Stub {
     }
 
     @Override
-    public int startOperation(int code, int uid, String packageName) {
+    public int startOperation(IBinder token, int code, int uid, String packageName) {
         verifyIncomingUid(uid);
         verifyIncomingOp(code);
+        ClientState client = (ClientState)token;
         synchronized (this) {
             Ops ops = getOpsLocked(uid, packageName, true);
             if (ops == null) {
@@ -475,32 +531,46 @@ public class AppOpsService extends IAppOpsService.Stub {
                 op.duration = -1;
             }
             op.nesting++;
+            if (client.mStartedOps != null) {
+                client.mStartedOps.add(op);
+            }
             return AppOpsManager.MODE_ALLOWED;
         }
     }
 
     @Override
-    public void finishOperation(int code, int uid, String packageName) {
+    public void finishOperation(IBinder token, int code, int uid, String packageName) {
         verifyIncomingUid(uid);
         verifyIncomingOp(code);
+        ClientState client = (ClientState)token;
         synchronized (this) {
             Op op = getOpLocked(code, uid, packageName, true);
             if (op == null) {
                 return;
             }
-            if (op.nesting <= 1) {
-                if (op.nesting == 1) {
-                    op.duration = (int)(System.currentTimeMillis() - op.time);
-                    op.time += op.duration;
-                } else {
-                    Slog.w(TAG, "Finishing op nesting under-run: uid " + uid + " pkg " + packageName
-                        + " code " + code + " time=" + op.time + " duration=" + op.duration
-                        + " nesting=" + op.nesting);
+            if (client.mStartedOps != null) {
+                if (!client.mStartedOps.remove(op)) {
+                    throw new IllegalStateException("Operation not started: uid" + op.uid
+                            + " pkg=" + op.packageName + " op=" + op.op);
                 }
-                op.nesting = 0;
+            }
+            finishOperationLocked(op);
+        }
+    }
+
+    void finishOperationLocked(Op op) {
+        if (op.nesting <= 1) {
+            if (op.nesting == 1) {
+                op.duration = (int)(System.currentTimeMillis() - op.time);
+                op.time += op.duration;
             } else {
-                op.nesting--;
+                Slog.w(TAG, "Finishing op nesting under-run: uid " + op.uid + " pkg "
+                        + op.packageName + " code " + op.op + " time=" + op.time
+                        + " duration=" + op.duration + " nesting=" + op.nesting);
             }
+            op.nesting = 0;
+        } else {
+            op.nesting--;
         }
     }
 
@@ -601,7 +671,7 @@ public class AppOpsService extends IAppOpsService.Stub {
             if (!edit) {
                 return null;
             }
-            op = new Op(code);
+            op = new Op(ops.uid, ops.packageName, code);
             ops.put(code, op);
         }
         if (edit) {
@@ -711,7 +781,7 @@ public class AppOpsService extends IAppOpsService.Stub {
 
             String tagName = parser.getName();
             if (tagName.equals("op")) {
-                Op op = new Op(Integer.parseInt(parser.getAttributeValue(null, "n")));
+                Op op = new Op(uid, pkgName, Integer.parseInt(parser.getAttributeValue(null, "n")));
                 String mode = parser.getAttributeValue(null, "m");
                 if (mode != null) {
                     op.mode = Integer.parseInt(mode);
@@ -831,6 +901,62 @@ public class AppOpsService extends IAppOpsService.Stub {
         synchronized (this) {
             pw.println("Current AppOps Service state:");
             final long now = System.currentTimeMillis();
+            boolean needSep = false;
+            if (mOpModeWatchers.size() > 0) {
+                needSep = true;
+                pw.println("  Op mode watchers:");
+                for (int i=0; i<mOpModeWatchers.size(); i++) {
+                    pw.print("    Op "); pw.print(AppOpsManager.opToName(mOpModeWatchers.keyAt(i)));
+                    pw.println(":");
+                    ArrayList<Callback> callbacks = mOpModeWatchers.valueAt(i);
+                    for (int j=0; j<callbacks.size(); j++) {
+                        pw.print("      #"); pw.print(j); pw.print(": ");
+                        pw.println(callbacks.get(j));
+                    }
+                }
+            }
+            if (mPackageModeWatchers.size() > 0) {
+                needSep = true;
+                pw.println("  Package mode watchers:");
+                for (int i=0; i<mPackageModeWatchers.size(); i++) {
+                    pw.print("    Pkg "); pw.print(mPackageModeWatchers.keyAt(i));
+                    pw.println(":");
+                    ArrayList<Callback> callbacks = mPackageModeWatchers.valueAt(i);
+                    for (int j=0; j<callbacks.size(); j++) {
+                        pw.print("      #"); pw.print(j); pw.print(": ");
+                        pw.println(callbacks.get(j));
+                    }
+                }
+            }
+            if (mModeWatchers.size() > 0) {
+                needSep = true;
+                pw.println("  All mode watchers:");
+                for (int i=0; i<mModeWatchers.size(); i++) {
+                    pw.print("    "); pw.print(mModeWatchers.keyAt(i));
+                    pw.print(" -> "); pw.println(mModeWatchers.valueAt(i));
+                }
+            }
+            if (mClients.size() > 0) {
+                needSep = true;
+                pw.println("  Clients:");
+                for (int i=0; i<mClients.size(); i++) {
+                    pw.print("    "); pw.print(mClients.keyAt(i)); pw.println(":");
+                    ClientState cs = mClients.valueAt(i);
+                    pw.print("      "); pw.println(cs);
+                    if (cs.mStartedOps != null && cs.mStartedOps.size() > 0) {
+                        pw.println("      Started ops:");
+                        for (int j=0; j<cs.mStartedOps.size(); j++) {
+                            Op op = cs.mStartedOps.get(j);
+                            pw.print("        "); pw.print("uid="); pw.print(op.uid);
+                            pw.print(" pkg="); pw.print(op.packageName);
+                            pw.print(" op="); pw.println(AppOpsManager.opToName(op.op));
+                        }
+                    }
+                }
+            }
+            if (needSep) {
+                pw.println();
+            }
             for (int i=0; i<mUidOps.size(); i++) {
                 pw.print("  Uid "); UserHandle.formatUid(pw, mUidOps.keyAt(i)); pw.println(":");
                 HashMap<String, Ops> pkgOps = mUidOps.valueAt(i);
diff --git a/services/java/com/android/server/VibratorService.java b/services/java/com/android/server/VibratorService.java
index 9b5f8f6..28eb948 100644
--- a/services/java/com/android/server/VibratorService.java
+++ b/services/java/com/android/server/VibratorService.java
@@ -342,7 +342,8 @@ public class VibratorService extends IVibratorService.Stub
     // Lock held on mVibrations
     private void startVibrationLocked(final Vibration vib) {
         try {
-            int mode = mAppOpsService.startOperation(AppOpsManager.OP_VIBRATE, vib.mUid, vib.mPackageName);
+            int mode = mAppOpsService.startOperation(AppOpsManager.getToken(mAppOpsService),
+                    AppOpsManager.OP_VIBRATE, vib.mUid, vib.mPackageName);
             if (mode != AppOpsManager.MODE_ALLOWED) {
                 if (mode == AppOpsManager.MODE_ERRORED) {
                     Slog.w(TAG, "Would be an error: vibrate from uid " + vib.mUid);
@@ -366,7 +367,8 @@ public class VibratorService extends IVibratorService.Stub
     private void reportFinishVibrationLocked() {
         if (mCurrentVibration != null) {
             try {
-                mAppOpsService.finishOperation(AppOpsManager.OP_VIBRATE, mCurrentVibration.mUid,
+                mAppOpsService.finishOperation(AppOpsManager.getToken(mAppOpsService),
+                        AppOpsManager.OP_VIBRATE, mCurrentVibration.mUid,
                         mCurrentVibration.mPackageName);
             } catch (RemoteException e) {
             }
diff --git a/services/java/com/android/server/location/GpsLocationProvider.java b/services/java/com/android/server/location/GpsLocationProvider.java
index 4791ec0..38453c8 100644
--- a/services/java/com/android/server/location/GpsLocationProvider.java
+++ b/services/java/com/android/server/location/GpsLocationProvider.java
@@ -892,7 +892,8 @@ public class GpsLocationProvider implements LocationProviderInterface {
             for (int i=0; i<newWork.size(); i++) {
                 try {
                     int uid = newWork.get(i);
-                    mAppOpsService.startOperation(AppOpsManager.OP_GPS, uid, newWork.getName(i));
+                    mAppOpsService.startOperation(AppOpsManager.getToken(mAppOpsService),
+                            AppOpsManager.OP_GPS, uid, newWork.getName(i));
                     if (uid != lastuid) {
                         lastuid = uid;
                         mBatteryStats.noteStartGps(uid);
@@ -909,7 +910,8 @@ public class GpsLocationProvider implements LocationProviderInterface {
             for (int i=0; i<goneWork.size(); i++) {
                 try {
                     int uid = goneWork.get(i);
-                    mAppOpsService.finishOperation(AppOpsManager.OP_GPS, uid, goneWork.getName(i));
+                    mAppOpsService.finishOperation(AppOpsManager.getToken(mAppOpsService),
+                            AppOpsManager.OP_GPS, uid, goneWork.getName(i));
                     if (uid != lastuid) {
                         lastuid = uid;
                         mBatteryStats.noteStopGps(uid);
diff --git a/services/java/com/android/server/power/Notifier.java b/services/java/com/android/server/power/Notifier.java
index e44cfe5..264e2e9 100644
--- a/services/java/com/android/server/power/Notifier.java
+++ b/services/java/com/android/server/power/Notifier.java
@@ -144,7 +144,8 @@ final class Notifier {
             } else {
                 mBatteryStats.noteStartWakelock(ownerUid, ownerPid, tag, monitorType);
                 // XXX need to deal with disabled operations.
-                mAppOps.startOperation(AppOpsManager.OP_WAKE_LOCK, ownerUid, packageName);
+                mAppOps.startOperation(AppOpsManager.getToken(mAppOps),
+                        AppOpsManager.OP_WAKE_LOCK, ownerUid, packageName);
             }
         } catch (RemoteException ex) {
             // Ignore
@@ -169,7 +170,8 @@ final class Notifier {
                 mBatteryStats.noteStopWakelockFromSource(workSource, ownerPid, tag, monitorType);
             } else {
                 mBatteryStats.noteStopWakelock(ownerUid, ownerPid, tag, monitorType);
-                mAppOps.finishOperation(AppOpsManager.OP_WAKE_LOCK, ownerUid, packageName);
+                mAppOps.finishOperation(AppOpsManager.getToken(mAppOps),
+                        AppOpsManager.OP_WAKE_LOCK, ownerUid, packageName);
             }
         } catch (RemoteException ex) {
             // Ignore