Fix issue #5214105 and work on issue #5109947.
Change-Id: Iff2cfec5280a314989d915aa830c16124f921611
5214105: taking a screenshot while "Android is upgrading..." crashes device
5109947: Race condition between retrieving a content provider and updating its oom adj
diff --git a/services/java/com/android/server/am/ActivityManagerService.java b/services/java/com/android/server/am/ActivityManagerService.java
index 7232a94..bb5e989 100644
--- a/services/java/com/android/server/am/ActivityManagerService.java
+++ b/services/java/com/android/server/am/ActivityManagerService.java
@@ -91,7 +91,6 @@
import android.os.FileObserver;
import android.os.FileUtils;
import android.os.Handler;
-import android.os.HandlerThread;
import android.os.IBinder;
import android.os.IPermissionController;
import android.os.Looper;
@@ -5518,6 +5517,48 @@
return msg;
}
+ boolean incProviderCount(ProcessRecord r, ContentProviderRecord cpr) {
+ if (r != null) {
+ Integer cnt = r.conProviders.get(cpr);
+ if (DEBUG_PROVIDER) Slog.v(TAG,
+ "Adding provider requested by "
+ + r.processName + " from process "
+ + cpr.info.processName + ": " + cpr.name.flattenToShortString()
+ + " cnt=" + (cnt == null ? 1 : cnt));
+ if (cnt == null) {
+ cpr.clients.add(r);
+ r.conProviders.put(cpr, new Integer(1));
+ return true;
+ } else {
+ r.conProviders.put(cpr, new Integer(cnt.intValue()+1));
+ }
+ } else {
+ cpr.externals++;
+ }
+ return false;
+ }
+
+ boolean decProviderCount(ProcessRecord r, ContentProviderRecord cpr) {
+ if (r != null) {
+ Integer cnt = r.conProviders.get(cpr);
+ if (DEBUG_PROVIDER) Slog.v(TAG,
+ "Removing provider requested by "
+ + r.processName + " from process "
+ + cpr.info.processName + ": " + cpr.name.flattenToShortString()
+ + " cnt=" + cnt);
+ if (cnt == null || cnt.intValue() <= 1) {
+ cpr.clients.remove(r);
+ r.conProviders.remove(cpr);
+ return true;
+ } else {
+ r.conProviders.put(cpr, new Integer(cnt.intValue()-1));
+ }
+ } else {
+ cpr.externals++;
+ }
+ return false;
+ }
+
private final ContentProviderHolder getContentProviderImpl(
IApplicationThread caller, String name) {
ContentProviderRecord cpr;
@@ -5537,7 +5578,8 @@
// First check if this content provider has been published...
cpr = mProvidersByName.get(name);
- if (cpr != null) {
+ boolean providerRunning = cpr != null;
+ if (providerRunning) {
cpi = cpr.info;
String msg;
if ((msg=checkContentProviderPermissionLocked(cpi, r)) != null) {
@@ -5561,18 +5603,8 @@
// In this case the provider instance already exists, so we can
// return it right away.
- if (r != null) {
- if (DEBUG_PROVIDER) Slog.v(TAG,
- "Adding provider requested by "
- + r.processName + " from process "
- + cpr.info.processName);
- Integer cnt = r.conProviders.get(cpr);
- if (cnt == null) {
- r.conProviders.put(cpr, new Integer(1));
- } else {
- r.conProviders.put(cpr, new Integer(cnt.intValue()+1));
- }
- cpr.clients.add(r);
+ final boolean countChanged = incProviderCount(r, cpr);
+ if (countChanged) {
if (cpr.app != null && r.setAdj <= ProcessList.PERCEPTIBLE_APP_ADJ) {
// If this is a perceptible app accessing the provider,
// make sure to count it as being accessed and thus
@@ -5580,17 +5612,46 @@
// content providers are often expensive to start.
updateLruProcessLocked(cpr.app, false, true);
}
- } else {
- cpr.externals++;
}
if (cpr.app != null) {
- updateOomAdjLocked(cpr.app);
+ if (false) {
+ if (cpr.name.flattenToShortString().equals(
+ "com.android.providers.calendar/.CalendarProvider2")) {
+ Slog.v(TAG, "****************** KILLING "
+ + cpr.name.flattenToShortString());
+ Process.killProcess(cpr.app.pid);
+ }
+ }
+ boolean success = updateOomAdjLocked(cpr.app);
+ if (DEBUG_PROVIDER) Slog.i(TAG, "Adjust success: " + success);
+ // NOTE: there is still a race here where a signal could be
+ // pending on the process even though we managed to update its
+ // adj level. Not sure what to do about this, but at least
+ // the race is now smaller.
+ if (!success) {
+ // Uh oh... it looks like the provider's process
+ // has been killed on us. We need to wait for a new
+ // process to be started, and make sure its death
+ // doesn't kill our process.
+ Slog.i(TAG,
+ "Existing provider " + cpr.name.flattenToShortString()
+ + " is crashing; detaching " + r);
+ boolean lastRef = decProviderCount(r, cpr);
+ appDiedLocked(cpr.app, cpr.app.pid, cpr.app.thread);
+ if (!lastRef) {
+ // This wasn't the last ref our process had on
+ // the provider... we have now been killed, bail.
+ return null;
+ }
+ providerRunning = false;
+ }
}
Binder.restoreCallingIdentity(origId);
+ }
- } else {
+ if (!providerRunning) {
try {
cpi = AppGlobals.getPackageManager().
resolveContentProvider(name,
@@ -5701,22 +5762,7 @@
mProvidersByClass.put(comp, cpr);
}
mProvidersByName.put(name, cpr);
-
- if (r != null) {
- if (DEBUG_PROVIDER) Slog.v(TAG,
- "Adding provider requested by "
- + r.processName + " from process "
- + cpr.info.processName);
- Integer cnt = r.conProviders.get(cpr);
- if (cnt == null) {
- r.conProviders.put(cpr, new Integer(1));
- } else {
- r.conProviders.put(cpr, new Integer(cnt.intValue()+1));
- }
- cpr.clients.add(r);
- } else {
- cpr.externals++;
- }
+ incProviderCount(r, cpr);
}
}
@@ -5780,24 +5826,16 @@
//update content provider record entry info
ComponentName comp = new ComponentName(cpr.info.packageName, cpr.info.name);
ContentProviderRecord localCpr = mProvidersByClass.get(comp);
- if (DEBUG_PROVIDER) Slog.v(TAG, "Removing provider requested by "
- + r.info.processName + " from process "
- + localCpr.appInfo.processName);
if (localCpr.app == r) {
//should not happen. taken care of as a local provider
Slog.w(TAG, "removeContentProvider called on local provider: "
+ cpr.info.name + " in process " + r.processName);
return;
} else {
- Integer cnt = r.conProviders.get(localCpr);
- if (cnt == null || cnt.intValue() <= 1) {
- localCpr.clients.remove(r);
- r.conProviders.remove(localCpr);
- } else {
- r.conProviders.put(localCpr, new Integer(cnt.intValue()-1));
+ if (decProviderCount(r, localCpr)) {
+ updateOomAdjLocked();
}
}
- updateOomAdjLocked();
}
}
@@ -13458,16 +13496,18 @@
}
}
- private final void updateOomAdjLocked(
+ private final boolean updateOomAdjLocked(
ProcessRecord app, int hiddenAdj, ProcessRecord TOP_APP) {
app.hiddenAdj = hiddenAdj;
if (app.thread == null) {
- return;
+ return false;
}
final boolean wasKeeping = app.keeping;
+ boolean success = true;
+
computeOomAdjLocked(app, hiddenAdj, TOP_APP, false);
if (app.curRawAdj != app.setRawAdj) {
@@ -13504,6 +13544,7 @@
" oom adj to " + app.curAdj + " because " + app.adjType);
app.setAdj = app.curAdj;
} else {
+ success = false;
Slog.w(TAG, "Failed setting oom adj of " + app + " to " + app.curAdj);
}
}
@@ -13518,6 +13559,7 @@
EventLog.writeEvent(EventLogTags.AM_KILL, app.pid,
app.processName, app.setAdj, app.waitingToKill);
Process.killProcessQuiet(app.pid);
+ success = false;
} else {
if (true) {
long oldId = Binder.clearCallingIdentity();
@@ -13540,6 +13582,7 @@
}
}
}
+ return success;
}
private final ActivityRecord resumedAppLocked() {
@@ -13553,7 +13596,7 @@
return resumedActivity;
}
- private final void updateOomAdjLocked(ProcessRecord app) {
+ private final boolean updateOomAdjLocked(ProcessRecord app) {
final ActivityRecord TOP_ACT = resumedAppLocked();
final ProcessRecord TOP_APP = TOP_ACT != null ? TOP_ACT.app : null;
int curAdj = app.curAdj;
@@ -13562,7 +13605,7 @@
mAdjSeq++;
- updateOomAdjLocked(app, app.hiddenAdj, TOP_APP);
+ boolean success = updateOomAdjLocked(app, app.hiddenAdj, TOP_APP);
final boolean nowHidden = app.curAdj >= ProcessList.HIDDEN_APP_MIN_ADJ
&& app.curAdj <= ProcessList.HIDDEN_APP_MAX_ADJ;
if (nowHidden != wasHidden) {
@@ -13570,6 +13613,7 @@
// list may also be changed.
updateOomAdjLocked();
}
+ return success;
}
final void updateOomAdjLocked() {