Merge "Freezer: fix binder race" into sc-qpr1-dev

services/core/java/com/android/server/am/CachedAppOptimizer.java

@@ -157,6 +157,9 @@ public final class CachedAppOptimizer {
     static final int SYNC_RECEIVED_WHILE_FROZEN = 1;
     static final int ASYNC_RECEIVED_WHILE_FROZEN = 2;
 
+    // Bitfield values for sync transactions received by frozen binder threads
+    static final int TXNS_PENDING_WHILE_FROZEN = 4;
+
     /**
      * This thread must be moved to the system background cpuset.
      * If that doesn't happen, it's probably going to draw a lot of power.
@@ -611,8 +614,9 @@ public final class CachedAppOptimizer {
      * binder for the specificed pid.
      *
      * @throws RuntimeException in case a flush/freeze operation could not complete successfully.
+     * @return 0 if success, or -EAGAIN indicating there's pending transaction.
      */
-    private static native void freezeBinder(int pid, boolean freeze);
+    private static native int freezeBinder(int pid, boolean freeze);
 
     /**
      * Retrieves binder freeze info about a process.
@@ -948,7 +952,7 @@ public final class CachedAppOptimizer {
             int freezeInfo = getBinderFreezeInfo(pid);
 
             if ((freezeInfo & SYNC_RECEIVED_WHILE_FROZEN) != 0) {
-                Slog.d(TAG_AM, "pid " + pid + " " + app.processName + " "
+                Slog.d(TAG_AM, "pid " + pid + " " + app.processName
                         + " received sync transactions while frozen, killing");
                 app.killLocked("Sync transaction while in frozen state",
                         ApplicationExitInfo.REASON_OTHER,
@@ -956,8 +960,8 @@ public final class CachedAppOptimizer {
                 processKilled = true;
             }
 
-            if ((freezeInfo & ASYNC_RECEIVED_WHILE_FROZEN) != 0) {
+            if ((freezeInfo & ASYNC_RECEIVED_WHILE_FROZEN) != 0 && DEBUG_FREEZER) {
-                Slog.d(TAG_AM, "pid " + pid + " " + app.processName + " "
+                Slog.d(TAG_AM, "pid " + pid + " " + app.processName
                         + " received async transactions while frozen");
             }
         } catch (Exception e) {
@@ -1292,7 +1296,9 @@ public final class CachedAppOptimizer {
         public void handleMessage(Message msg) {
             switch (msg.what) {
                 case SET_FROZEN_PROCESS_MSG:
+                    synchronized (mAm) {
                         freezeProcess((ProcessRecord) msg.obj);
+                    }
                     break;
                 case REPORT_UNFREEZE_MSG:
                     int pid = msg.arg1;
@@ -1306,6 +1312,15 @@ public final class CachedAppOptimizer {
             }
         }
 
+        @GuardedBy({"mAm", "mProcLock"})
+        private void rescheduleFreeze(final ProcessRecord proc, final String reason) {
+            Slog.d(TAG_AM, "Reschedule freeze for process " + proc.getPid()
+                    + " " + proc.processName + " (" + reason + ")");
+            unfreezeAppLSP(proc);
+            freezeAppAsyncLSP(proc);
+        }
+
+        @GuardedBy({"mAm"})
         private void freezeProcess(final ProcessRecord proc) {
             int pid = proc.getPid(); // Unlocked intentionally
             final String name = proc.processName;
@@ -1355,10 +1370,15 @@ public final class CachedAppOptimizer {
                     return;
                 }
 
+                Slog.d(TAG_AM, "freezing " + pid + " " + name);
+
                 // Freeze binder interface before the process, to flush any
                 // transactions that might be pending.
                 try {
-                    freezeBinder(pid, true);
+                    if (freezeBinder(pid, true) != 0) {
+                        rescheduleFreeze(proc, "outstanding txns");
+                        return;
+                    }
                 } catch (RuntimeException e) {
                     Slog.e(TAG_AM, "Unable to freeze binder for " + pid + " " + name);
                     mFreezeHandler.post(() -> {
@@ -1402,28 +1422,40 @@ public final class CachedAppOptimizer {
                         unfrozenDuration);
             }
 
+            try {
+                // post-check to prevent races
+                int freezeInfo = getBinderFreezeInfo(pid);
+
+                if ((freezeInfo & TXNS_PENDING_WHILE_FROZEN) != 0) {
+                    synchronized (mProcLock) {
+                        rescheduleFreeze(proc, "new pending txns");
+                    }
+                    return;
+                }
+            } catch (RuntimeException e) {
+                Slog.e(TAG_AM, "Unable to freeze binder for " + pid + " " + name);
+                mFreezeHandler.post(() -> {
+                    synchronized (mAm) {
+                        proc.killLocked("Unable to freeze binder interface",
+                                ApplicationExitInfo.REASON_OTHER,
+                                ApplicationExitInfo.SUBREASON_FREEZER_BINDER_IOCTL, true);
+                    }
+                });
+            }
+
             try {
                 // post-check to prevent races
                 if (mProcLocksReader.hasFileLocks(pid)) {
                     if (DEBUG_FREEZER) {
                         Slog.d(TAG_AM, name + " (" + pid + ") holds file locks, reverting freeze");
                     }
-
-                    synchronized (mAm) {
-                        synchronized (mProcLock) {
                     unfreezeAppLSP(proc);
                 }
-                    }
-                }
             } catch (Exception e) {
                 Slog.e(TAG_AM, "Unable to check file locks for " + name + "(" + pid + "): " + e);
-                synchronized (mAm) {
-                    synchronized (mProcLock) {
                 unfreezeAppLSP(proc);
             }
         }
-            }
-        }
 
         private void reportUnfreeze(int pid, int frozenDuration, String processName) {
 

services/core/jni/com_android_server_am_CachedAppOptimizer.cpp

@@ -55,6 +55,7 @@ using android::base::unique_fd;
 
 #define SYNC_RECEIVED_WHILE_FROZEN (1)
 #define ASYNC_RECEIVED_WHILE_FROZEN (2)
+#define TXNS_PENDING_WHILE_FROZEN (4)
 
 namespace android {
 
@@ -232,17 +233,20 @@ static void com_android_server_am_CachedAppOptimizer_compactProcess(JNIEnv*, job
     compactProcessOrFallback(pid, compactionFlags);
 }
 
-static void com_android_server_am_CachedAppOptimizer_freezeBinder(
+static jint com_android_server_am_CachedAppOptimizer_freezeBinder(
         JNIEnv *env, jobject clazz, jint pid, jboolean freeze) {
 
-    if (IPCThreadState::freeze(pid, freeze, 100 /* timeout [ms] */) != 0) {
+    jint retVal = IPCThreadState::freeze(pid, freeze, 100 /* timeout [ms] */);
+    if (retVal != 0 && retVal != -EAGAIN) {
         jniThrowException(env, "java/lang/RuntimeException", "Unable to freeze/unfreeze binder");
     }
+
+    return retVal;
 }
 
 static jint com_android_server_am_CachedAppOptimizer_getBinderFreezeInfo(JNIEnv *env,
         jobject clazz, jint pid) {
-    bool syncReceived = false, asyncReceived = false;
+    uint32_t syncReceived = 0, asyncReceived = 0;
 
     int error = IPCThreadState::getProcessFreezeInfo(pid, &syncReceived, &asyncReceived);
 
@@ -252,13 +256,12 @@ static jint com_android_server_am_CachedAppOptimizer_getBinderFreezeInfo(JNIEnv
 
     jint retVal = 0;
 
-    if(syncReceived) {
+    // bit 0 of sync_recv goes to bit 0 of retVal
-        retVal |= SYNC_RECEIVED_WHILE_FROZEN;;
+    retVal |= syncReceived & SYNC_RECEIVED_WHILE_FROZEN;
-    }
+    // bit 0 of async_recv goes to bit 1 of retVal
-
+    retVal |= (asyncReceived << 1) & ASYNC_RECEIVED_WHILE_FROZEN;
-    if(asyncReceived) {
+    // bit 1 of sync_recv goes to bit 2 of retVal
-        retVal |= ASYNC_RECEIVED_WHILE_FROZEN;
+    retVal |= (syncReceived << 1) & TXNS_PENDING_WHILE_FROZEN;
-    }
 
     return retVal;
 }
@@ -278,7 +281,7 @@ static const JNINativeMethod sMethods[] = {
         /* name, signature, funcPtr */
         {"compactSystem", "()V", (void*)com_android_server_am_CachedAppOptimizer_compactSystem},
         {"compactProcess", "(II)V", (void*)com_android_server_am_CachedAppOptimizer_compactProcess},
-        {"freezeBinder", "(IZ)V", (void*)com_android_server_am_CachedAppOptimizer_freezeBinder},
+        {"freezeBinder", "(IZ)I", (void*)com_android_server_am_CachedAppOptimizer_freezeBinder},
         {"getBinderFreezeInfo", "(I)I",
          (void*)com_android_server_am_CachedAppOptimizer_getBinderFreezeInfo},
         {"getFreezerCheckPath", "()Ljava/lang/String;",