Reduce race when connecting to an existing content provider.

We lost the code that checks to see if the target process still
exists and aborts trying to use it if so.  To reduce the race
there, we have a new explicit check of the state of the process.

Hopefully fixes some of issue #28737082.

Change-Id: I37a7a6e9767516d564168ef5e110c4adafe3fb76

4 files changed, 55 insertions, 4 deletions

diff --git a/core/java/android/os/Process.java b/core/java/android/os/Process.java
index 4506f51fe38a..f664e70cf7be 100644
--- a/core/java/android/os/Process.java
+++ b/core/java/android/os/Process.java
@@ -1184,6 +1184,8 @@ public class Process {
     /** @hide */
     public static final int PROC_QUOTES = 0x400;
     /** @hide */
+    public static final int PROC_CHAR = 0x800;
+    /** @hide */
     public static final int PROC_OUT_STRING = 0x1000;
     /** @hide */
     public static final int PROC_OUT_LONG = 0x2000;
diff --git a/core/jni/android_util_Process.cpp b/core/jni/android_util_Process.cpp
index f7a5e8a07fc0..3d952b0f713f 100644
--- a/core/jni/android_util_Process.cpp
+++ b/core/jni/android_util_Process.cpp
@@ -832,6 +832,7 @@ enum {
     PROC_COMBINE = 0x100,
     PROC_PARENS = 0x200,
     PROC_QUOTES = 0x400,
+    PROC_CHAR = 0x800,
     PROC_OUT_STRING = 0x1000,
     PROC_OUT_LONG = 0x2000,
     PROC_OUT_FLOAT = 0x4000,
@@ -933,8 +934,13 @@ jboolean android_os_Process_parseProcLineArray(JNIEnv* env, jobject clazz,
                 floatsData[di] = strtof(buffer+start, &end);
             }
             if ((mode&PROC_OUT_LONG) != 0 && di < NL) {
-                char* end;
-                longsData[di] = strtoll(buffer+start, &end, 10);
+                if ((mode&PROC_CHAR) != 0) {
+                    // Caller wants single first character returned as one long.
+                    longsData[di] = buffer[start];
+                } else {
+                    char* end;
+                    longsData[di] = strtoll(buffer+start, &end, 10);
+                }
             }
             if ((mode&PROC_OUT_STRING) != 0 && di < NS) {
                 jstring str = env->NewStringUTF(buffer+start);
diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java
index e9841129c4c3..c1a9d8555df9 100644
--- a/services/core/java/com/android/server/am/ActivityManagerService.java
+++ b/services/core/java/com/android/server/am/ActivityManagerService.java
@@ -273,6 +273,10 @@ import static android.content.pm.PackageManager.MATCH_SYSTEM_ONLY;
 import static android.content.pm.PackageManager.MATCH_UNINSTALLED_PACKAGES;
 import static android.content.pm.PackageManager.PERMISSION_GRANTED;
 import static android.content.res.Configuration.UI_MODE_TYPE_TELEVISION;
+import static android.os.Process.PROC_CHAR;
+import static android.os.Process.PROC_OUT_LONG;
+import static android.os.Process.PROC_PARENS;
+import static android.os.Process.PROC_SPACE_TERM;
 import static android.provider.Settings.Global.ALWAYS_FINISH_ACTIVITIES;
 import static android.provider.Settings.Global.DEBUG_APP;
 import static android.provider.Settings.Global.DEVELOPMENT_ENABLE_FREEFORM_WINDOWS_SUPPORT;
@@ -6412,7 +6416,7 @@ public final class ActivityManagerService extends ActivityManagerNative
         EventLog.writeEvent(EventLogTags.AM_PROC_BOUND, app.userId, app.pid, app.processName);
 
         app.makeActive(thread, mProcessStats);
-        app.curAdj = app.setAdj = ProcessList.INVALID_ADJ;
+        app.curAdj = app.setAdj = app.verifiedAdj = ProcessList.INVALID_ADJ;
         app.curSchedGroup = app.setSchedGroup = ProcessList.SCHED_GROUP_DEFAULT;
         app.forcingToForeground = null;
         updateProcessForegroundLocked(app, false, false);
@@ -10535,6 +10539,30 @@ public final class ActivityManagerService extends ActivityManagerNative
         }
     }
 
+    private static final int[] PROCESS_STATE_STATS_FORMAT = new int[] {
+            PROC_SPACE_TERM,
+            PROC_SPACE_TERM|PROC_PARENS,
+            PROC_SPACE_TERM|PROC_CHAR|PROC_OUT_LONG,        // 3: process state
+    };
+
+    private final long[] mProcessStateStatsLongs = new long[1];
+
+    boolean isProcessAliveLocked(ProcessRecord proc) {
+        if (proc.procStatFile == null) {
+            proc.procStatFile = "/proc/" + proc.pid + "/stat";
+        }
+        mProcessStateStatsLongs[0] = 0;
+        if (!Process.readProcFile(proc.procStatFile, PROCESS_STATE_STATS_FORMAT, null,
+                mProcessStateStatsLongs, null)) {
+            if (DEBUG_OOM_ADJ) Slog.d(TAG, "UNABLE TO RETRIEVE STATE FOR " + proc.procStatFile);
+            return false;
+        }
+        final long state = mProcessStateStatsLongs[0];
+        if (DEBUG_OOM_ADJ) Slog.d(TAG, "RETRIEVED STATE FOR " + proc.procStatFile + ": "
+                + (char)state);
+        return state != 'Z' && state != 'X' && state != 'x' && state != 'K';
+    }
+
     private ContentProviderHolder getContentProviderImpl(IApplicationThread caller,
             String name, IBinder token, boolean stable, int userId) {
         ContentProviderRecord cpr;
@@ -10622,7 +10650,16 @@ public final class ActivityManagerService extends ActivityManagerNative
                 }
 
                 checkTime(startTime, "getContentProviderImpl: before updateOomAdj");
+                final int verifiedAdj = cpr.proc.verifiedAdj;
                 boolean success = updateOomAdjLocked(cpr.proc);
+                // XXX things have changed so updateOomAdjLocked doesn't actually tell us
+                // if the process has been successfully adjusted.  So to reduce races with
+                // it, we will check whether the process still exists.  Note that this doesn't
+                // completely get rid of races with LMK killing the process, but should make
+                // them much smaller.
+                if (success && verifiedAdj != cpr.proc.setAdj && !isProcessAliveLocked(cpr.proc)) {
+                    success = false;
+                }
                 maybeUpdateProviderUsageStatsLocked(r, cpr.info.packageName, name);
                 checkTime(startTime, "getContentProviderImpl: after updateOomAdj");
                 if (DEBUG_PROVIDER) Slog.i(TAG_PROVIDER, "Adjust success: " + success);
@@ -10648,6 +10685,8 @@ public final class ActivityManagerService extends ActivityManagerNative
                     }
                     providerRunning = false;
                     conn = null;
+                } else {
+                    cpr.proc.verifiedAdj = cpr.proc.setAdj;
                 }
 
                 Binder.restoreCallingIdentity(origId);
@@ -20069,6 +20108,7 @@ public final class ActivityManagerService extends ActivityManagerNative
                     "Set " + app.pid + " " + app.processName + " adj " + app.curAdj + ": "
                     + app.adjType);
             app.setAdj = app.curAdj;
+            app.verifiedAdj = ProcessList.INVALID_ADJ;
         }
 
         if (app.setSchedGroup != app.curSchedGroup) {
diff --git a/services/core/java/com/android/server/am/ProcessRecord.java b/services/core/java/com/android/server/am/ProcessRecord.java
index 691fd2abe0b3..8911a3e94979 100644
--- a/services/core/java/com/android/server/am/ProcessRecord.java
+++ b/services/core/java/com/android/server/am/ProcessRecord.java
@@ -75,6 +75,7 @@ final class ProcessRecord {
     ProcessState baseProcessTracker;
     BatteryStatsImpl.Uid.Proc curProcBatteryStats;
     int pid;                    // The process of this application; 0 if none
+    String procStatFile;        // path to /proc/<pid>/stat
     int[] gids;                 // The gids this process was launched with
     String requiredAbi;         // The ABI this process was launched with
     String instructionSet;      // The instruction set this process was launched with
@@ -93,6 +94,7 @@ final class ProcessRecord {
     int setRawAdj;              // Last set OOM unlimited adjustment for this process
     int curAdj;                 // Current OOM adjustment for this process
     int setAdj;                 // Last set OOM adjustment for this process
+    int verifiedAdj;            // The last adjustment that was verified as actually being set
     int curSchedGroup;          // Currently desired scheduling class
     int setSchedGroup;          // Last set to background scheduling class
     int trimMemoryLevel;        // Last selected memory trimming level
@@ -441,7 +443,7 @@ final class ProcessRecord {
         pkgList.put(_info.packageName, new ProcessStats.ProcessStateHolder(_info.versionCode));
         maxAdj = ProcessList.UNKNOWN_ADJ;
         curRawAdj = setRawAdj = ProcessList.INVALID_ADJ;
-        curAdj = setAdj = ProcessList.INVALID_ADJ;
+        curAdj = setAdj = verifiedAdj = ProcessList.INVALID_ADJ;
         persistent = false;
         removed = false;
         lastStateTime = lastPssTime = nextPssTime = SystemClock.uptimeMillis();
@@ -449,6 +451,7 @@ final class ProcessRecord {
 
     public void setPid(int _pid) {
         pid = _pid;
+        procStatFile = null;
         shortStringName = null;
         stringName = null;
     }