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
diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java
index e984112..c1a9d85 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_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 @@
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 @@
}
}
+ 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 @@
}
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 @@
}
providerRunning = false;
conn = null;
+ } else {
+ cpr.proc.verifiedAdj = cpr.proc.setAdj;
}
Binder.restoreCallingIdentity(origId);
@@ -20069,6 +20108,7 @@
"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 691fd2a..8911a3e 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 @@
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 @@
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 @@
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 @@
public void setPid(int _pid) {
pid = _pid;
+ procStatFile = null;
shortStringName = null;
stringName = null;
}