From a590d2be935ef502943a1e6615500aa10e67c85a Mon Sep 17 00:00:00 2001 From: Dianne Hackborn Date: Mon, 27 Jun 2016 15:07:18 -0700 Subject: [PATCH] Fix bug where process whitelist manager state would not be correct. We can't update this in updateOomAdjLocked(), because we very deliberately only iterate through services in there as needed. The correct thing to do is update the process as services/connections are associated with it, so do that. Now basically all of the logic for tracking the state is in ActiveServices, as we bind and unbind services and add and removing them from process records. It's a little messy because we don't have a central place for removing them from process records, which should be cleaned up in the future. Part of fixes for issue #29480440 Change-Id: Iac96f002a5b4e3b0277df244ff7b90f59a6e8440 --- .../java/com/android/server/am/ActiveServices.java | 41 +++++++++++++++++++++- .../android/server/am/ActivityManagerService.java | 4 --- .../java/com/android/server/am/ServiceRecord.java | 17 +++++++++ 3 files changed, 57 insertions(+), 5 deletions(-) diff --git a/services/core/java/com/android/server/am/ActiveServices.java b/services/core/java/com/android/server/am/ActiveServices.java index 798662910cbf..d9148817881b 100755 --- a/services/core/java/com/android/server/am/ActiveServices.java +++ b/services/core/java/com/android/server/am/ActiveServices.java @@ -43,7 +43,6 @@ import android.os.TransactionTooLargeException; import android.util.ArrayMap; import android.util.ArraySet; -import com.android.internal.app.procstats.ProcessStats; import com.android.internal.app.procstats.ServiceState; import com.android.internal.os.BatteryStatsImpl; import com.android.internal.os.TransferPipe; @@ -751,6 +750,17 @@ public final class ActiveServices { mAm.updateProcessForegroundLocked(proc, anyForeground, oomAdj); } + private void updateWhitelistManagerLocked(ProcessRecord proc) { + proc.whitelistManager = false; + for (int i=proc.services.size()-1; i>=0; i--) { + ServiceRecord sr = proc.services.valueAt(i); + if (sr.whitelistManager) { + proc.whitelistManager = true; + break; + } + } + } + public void updateServiceConnectionActivitiesLocked(ProcessRecord clientProc) { ArraySet updatedProcesses = null; for (int i = 0; i < clientProc.connections.size(); i++) { @@ -997,6 +1007,9 @@ public final class ActiveServices { if ((c.flags&Context.BIND_ABOVE_CLIENT) != 0) { b.client.hasAboveClient = true; } + if ((c.flags&Context.BIND_ALLOW_WHITELIST_MANAGEMENT) != 0) { + s.whitelistManager = true; + } if (s.app != null) { updateServiceClientActivitiesLocked(s.app, c, true); } @@ -1019,6 +1032,9 @@ public final class ActiveServices { if ((flags&Context.BIND_TREAT_LIKE_ACTIVITY) != 0) { s.app.treatLikeActivity = true; } + if (s.whitelistManager) { + s.app.whitelistManager = true; + } // This could have made the service more important. mAm.updateLruProcessLocked(s.app, s.app.hasClientActivities || s.app.treatLikeActivity, b.client); @@ -1807,6 +1823,10 @@ public final class ActiveServices { } } + if (r.whitelistManager) { + app.whitelistManager = true; + } + requestServiceBindingsLocked(r, execInFg); updateServiceClientActivitiesLocked(app, null, true); @@ -2018,6 +2038,9 @@ public final class ActiveServices { r.stats.stopLaunchedLocked(); } r.app.services.remove(r); + if (r.whitelistManager) { + updateWhitelistManagerLocked(r.app); + } if (r.app.thread != null) { updateServiceForegroundLocked(r.app, false); try { @@ -2085,6 +2108,14 @@ public final class ActiveServices { if ((c.flags&Context.BIND_ABOVE_CLIENT) != 0) { b.client.updateHasAboveClientLocked(); } + // If this connection requested whitelist management, see if we should + // now clear that state. + if ((c.flags&Context.BIND_ALLOW_WHITELIST_MANAGEMENT) != 0) { + s.updateWhitelistManager(); + if (!s.whitelistManager && s.app != null) { + updateWhitelistManagerLocked(s.app); + } + } if (s.app != null) { updateServiceClientActivitiesLocked(s.app, c, true); } @@ -2284,6 +2315,9 @@ public final class ActiveServices { if (finishing) { if (r.app != null && !r.app.persistent) { r.app.services.remove(r); + if (r.whitelistManager) { + updateWhitelistManagerLocked(r.app); + } } r.app = null; } @@ -2379,6 +2413,9 @@ public final class ActiveServices { service.app.removed = killProcess; if (!service.app.persistent) { service.app.services.remove(service); + if (service.whitelistManager) { + updateWhitelistManagerLocked(service.app); + } } } service.app = null; @@ -2497,6 +2534,8 @@ public final class ActiveServices { updateServiceConnectionActivitiesLocked(app); app.connections.clear(); + app.whitelistManager = false; + // Clear app state from services. for (int i = app.services.size() - 1; i >= 0; i--) { ServiceRecord sr = app.services.valueAt(i); diff --git a/services/core/java/com/android/server/am/ActivityManagerService.java b/services/core/java/com/android/server/am/ActivityManagerService.java index d911f52e5de1..08b204af7fe7 100644 --- a/services/core/java/com/android/server/am/ActivityManagerService.java +++ b/services/core/java/com/android/server/am/ActivityManagerService.java @@ -19294,7 +19294,6 @@ public final class ActivityManagerService extends ActivityManagerNative } boolean mayBeTop = false; - app.whitelistManager = false; for (int is = app.services.size()-1; is >= 0 && (adj > ProcessList.FOREGROUND_APP_ADJ @@ -19353,9 +19352,6 @@ public final class ActivityManagerService extends ActivityManagerNative // Binding to ourself is not interesting. continue; } - if ((cr.flags & Context.BIND_ALLOW_WHITELIST_MANAGEMENT) != 0) { - app.whitelistManager = true; - } if ((cr.flags&Context.BIND_WAIVE_PRIORITY) == 0) { ProcessRecord client = cr.binding.client; diff --git a/services/core/java/com/android/server/am/ServiceRecord.java b/services/core/java/com/android/server/am/ServiceRecord.java index 0a081e9809ea..2bfc4021f887 100644 --- a/services/core/java/com/android/server/am/ServiceRecord.java +++ b/services/core/java/com/android/server/am/ServiceRecord.java @@ -90,6 +90,7 @@ final class ServiceRecord extends Binder { ProcessRecord isolatedProc; // keep track of isolated process, if requested ServiceState tracker; // tracking service execution, may be null ServiceState restartTracker; // tracking service restart + boolean whitelistManager; // any bindings to this service have BIND_ALLOW_WHITELIST_MANAGEMENT? boolean delayed; // are we waiting to start this service in the background? boolean isForeground; // is service currently in foreground mode? int foregroundId; // Notification ID of last foreground req. @@ -225,6 +226,9 @@ final class ServiceRecord extends Binder { if (isolatedProc != null) { pw.print(prefix); pw.print("isolatedProc="); pw.println(isolatedProc); } + if (whitelistManager) { + pw.print(prefix); pw.print("whitelistManager="); pw.println(whitelistManager); + } if (delayed) { pw.print(prefix); pw.print("delayed="); pw.println(delayed); } @@ -391,6 +395,19 @@ final class ServiceRecord extends Binder { return false; } + public void updateWhitelistManager() { + whitelistManager = false; + for (int conni=connections.size()-1; conni>=0; conni--) { + ArrayList cr = connections.valueAt(conni); + for (int i=0; i