From 130b0d2b2629bdd8fc415e0f3da947f965a3f29d Mon Sep 17 00:00:00 2001 From: Dianne Hackborn Date: Tue, 26 Jul 2011 22:07:48 -0700 Subject: Fix issue #4466531: onServiceConnected() not called after... ...apk reinstall; affects user privacy Disconnecting a ServiceConnection after an app is torn down could impact the bookkeeping of the same service if it has been started for the app. Also address issue #5073927: GSF process can't be killed A new flag allows the systems location manager service to tell the activity manager to not pull bound services up forever into the visible adj level. Change-Id: I2557eca0e4bd48f3b10007c40ec878e769fd96a8 --- core/java/android/content/Context.java | 12 +- .../android/server/am/ActivityManagerService.java | 127 +++++++++++++-------- .../com/android/server/am/ConnectionRecord.java | 4 + .../com/android/server/location/GeocoderProxy.java | 10 +- .../server/location/LocationProviderProxy.java | 11 +- 5 files changed, 109 insertions(+), 55 deletions(-) diff --git a/core/java/android/content/Context.java b/core/java/android/content/Context.java index cdda910d5fb2..2a024466f9fd 100644 --- a/core/java/android/content/Context.java +++ b/core/java/android/content/Context.java @@ -129,7 +129,7 @@ public abstract class Context { /** * Flag for {@link #bindService}: don't allow this binding to raise * the target service's process to the foreground scheduling priority. - * It will still be raised to the at least the same memory priority + * It will still be raised to at least the same memory priority * as the client (so that its process will not be killable in any * situation where the client is not killable), but for CPU scheduling * purposes it may be left in the background. This only has an impact @@ -138,6 +138,16 @@ public abstract class Context { */ public static final int BIND_NOT_FOREGROUND = 0x0004; + /** + * Flag for {@link #bindService}: allow the process hosting the bound + * service to go through its normal memory management. It will be + * treated more like a running service, allowing the system to + * (temporarily) expunge the process if low on memory or for some other + * whim it may have. + * @hide + */ + public static final int BIND_ALLOW_OOM_MANAGEMENT = 0x0008; + /** Return an AssetManager instance for your application's package. */ public abstract AssetManager getAssets(); diff --git a/services/java/com/android/server/am/ActivityManagerService.java b/services/java/com/android/server/am/ActivityManagerService.java index 3389f339a7f2..8835314627f1 100644 --- a/services/java/com/android/server/am/ActivityManagerService.java +++ b/services/java/com/android/server/am/ActivityManagerService.java @@ -1821,7 +1821,7 @@ public final class ActivityManagerService extends ActivityManagerNative // An application record is attached to a previous process, // clean it up now. if (DEBUG_PROCESSES) Slog.v(TAG, "App died: " + app); - handleAppDiedLocked(app, true); + handleAppDiedLocked(app, true, true); } } @@ -2658,8 +2658,8 @@ public final class ActivityManagerService extends ActivityManagerNative * to the process. */ private final void handleAppDiedLocked(ProcessRecord app, - boolean restarting) { - cleanUpApplicationRecordLocked(app, restarting, -1); + boolean restarting, boolean allowRestart) { + cleanUpApplicationRecordLocked(app, restarting, allowRestart, -1); if (!restarting) { mLruProcesses.remove(app); } @@ -2791,7 +2791,7 @@ public final class ActivityManagerService extends ActivityManagerNative TAG, "Dying app: " + app + ", pid: " + pid + ", thread: " + thread.asBinder()); boolean doLowMem = app.instrumentationClass == null; - handleAppDiedLocked(app, false); + handleAppDiedLocked(app, false, true); if (doLowMem) { // If there are no longer any background processes running, @@ -3195,7 +3195,7 @@ public final class ActivityManagerService extends ActivityManagerNative return; } killPackageProcessesLocked(packageName, pkgUid, - SECONDARY_SERVER_ADJ, false, true); + SECONDARY_SERVER_ADJ, false, true, true); } } finally { Binder.restoreCallingIdentity(callingId); @@ -3358,7 +3358,7 @@ public final class ActivityManagerService extends ActivityManagerNative } private final boolean killPackageProcessesLocked(String packageName, int uid, - int minOomAdj, boolean callerWillRestart, boolean doit) { + int minOomAdj, boolean callerWillRestart, boolean allowRestart, boolean doit) { ArrayList procs = new ArrayList(); // Remove all processes this package may have touched: all with the @@ -3389,7 +3389,7 @@ public final class ActivityManagerService extends ActivityManagerNative int N = procs.size(); for (int i=0; i 0; } @@ -3419,7 +3419,7 @@ public final class ActivityManagerService extends ActivityManagerNative } boolean didSomething = killPackageProcessesLocked(name, uid, -100, - callerWillRestart, doit); + callerWillRestart, false, doit); for (i=mMainStack.mHistory.size()-1; i>=0; i--) { ActivityRecord r = (ActivityRecord)mMainStack.mHistory.get(i); @@ -3471,7 +3471,8 @@ public final class ActivityManagerService extends ActivityManagerNative return didSomething; } - private final boolean removeProcessLocked(ProcessRecord app, boolean callerWillRestart) { + private final boolean removeProcessLocked(ProcessRecord app, + boolean callerWillRestart, boolean allowRestart) { final String name = app.processName; final int uid = app.info.uid; if (DEBUG_PROCESSES) Slog.d( @@ -3490,7 +3491,7 @@ public final class ActivityManagerService extends ActivityManagerNative mPidsSelfLocked.remove(pid); mHandler.removeMessages(PROC_START_TIMEOUT_MSG, app); } - handleAppDiedLocked(app, true); + handleAppDiedLocked(app, true, allowRestart); mLruProcesses.remove(app); Process.killProcess(pid); @@ -3600,7 +3601,7 @@ public final class ActivityManagerService extends ActivityManagerNative // If this application record is still attached to a previous // process, clean it up now. if (app.thread != null) { - handleAppDiedLocked(app, true); + handleAppDiedLocked(app, true, true); } // Tell the process all about itself. @@ -3783,7 +3784,7 @@ public final class ActivityManagerService extends ActivityManagerNative if (badApp) { // todo: Also need to kill application to deal with all // kinds of exceptions. - handleAppDiedLocked(app, false); + handleAppDiedLocked(app, false, true); return false; } @@ -6643,7 +6644,7 @@ public final class ActivityManagerService extends ActivityManagerNative for (int i=procsToKill.size()-1; i>=0; i--) { ProcessRecord proc = procsToKill.get(i); Slog.i(TAG, "Removing system update proc: " + proc); - removeProcessLocked(proc, true); + removeProcessLocked(proc, true, false); } } @@ -6826,10 +6827,6 @@ public final class ActivityManagerService extends ActivityManagerNative } } if (!app.persistent) { - // Don't let services in this process be restarted and potentially - // annoy the user repeatedly. Unless it is persistent, since those - // processes run critical code. - killServicesLocked(app, false); // We don't want to start this process again until the user // explicitly does so... but for persistent process, we really // need to keep it running. If a persistent process is actually @@ -6840,7 +6837,10 @@ public final class ActivityManagerService extends ActivityManagerNative app.bad = true; mProcessCrashTimes.remove(app.info.processName, app.info.uid); app.removed = true; - removeProcessLocked(app, false); + // Don't let services in this process be restarted and potentially + // annoy the user repeatedly. Unless it is persistent, since those + // processes run critical code. + removeProcessLocked(app, false, false); mMainStack.resumeTopActivityLocked(null); return false; } @@ -9120,7 +9120,7 @@ public final class ActivityManagerService extends ActivityManagerNative // Should the service remain running? Note that in the // extreme case of so many attempts to deliver a command - // that it failed, that we also will stop it here. + // that it failed we also will stop it here. if (sr.startRequested && (sr.stopIfKilled || canceled)) { if (sr.pendingStarts.size() == 0) { sr.startRequested = false; @@ -9189,7 +9189,7 @@ public final class ActivityManagerService extends ActivityManagerNative * a process when running in single process mode. */ private final void cleanUpApplicationRecordLocked(ProcessRecord app, - boolean restarting, int index) { + boolean restarting, boolean allowRestart, int index) { if (index >= 0) { mLruProcesses.remove(index); } @@ -9221,7 +9221,7 @@ public final class ActivityManagerService extends ActivityManagerNative app.foregroundActivities = false; app.hasShownUi = false; - killServicesLocked(app, true); + killServicesLocked(app, allowRestart); boolean restart = false; @@ -9238,7 +9238,7 @@ public final class ActivityManagerService extends ActivityManagerNative // See if someone is waiting for this provider... in which // case we don't remove it, but just let it restart. int i = 0; - if (!app.bad) { + if (!app.bad && allowRestart) { for (; i c = it.next(); for (int i=0; i client.hiddenAdj) { if (client.hiddenAdj >= VISIBLE_APP_ADJ) { @@ -12782,8 +12789,35 @@ public final class ActivityManagerService extends ActivityManagerNative myHiddenAdj = VISIBLE_APP_ADJ; } } - int clientAdj = computeOomAdjLocked( + clientAdj = computeOomAdjLocked( client, myHiddenAdj, TOP_APP, true); + String adjType = null; + if ((cr.flags&Context.BIND_ALLOW_OOM_MANAGEMENT) != 0) { + // Not doing bind OOM management, so treat + // this guy more like a started service. + if (app.hasShownUi) { + // If this process has shown some UI, let it immediately + // go to the LRU list because it may be pretty heavy with + // UI stuff. We'll tag it with a label just to help + // debug and understand what is going on. + if (adj > clientAdj) { + adjType = "bound-bg-ui-services"; + } + clientAdj = adj; + } else { + if (now >= (s.lastActivity+MAX_SERVICE_INACTIVITY)) { + // This service has not seen activity within + // recent memory, so allow it to drop to the + // LRU list if there is no other reason to keep + // it around. We'll also tag it with a label just + // to help debug and undertand what is going on. + if (adj > clientAdj) { + adjType = "bound-bg-services"; + } + clientAdj = adj; + } + } + } if (adj > clientAdj) { adj = clientAdj >= VISIBLE_APP_ADJ ? clientAdj : VISIBLE_APP_ADJ; @@ -12793,7 +12827,10 @@ public final class ActivityManagerService extends ActivityManagerNative if (client.keeping) { app.keeping = true; } - app.adjType = "service"; + adjType = "service"; + } + if (adjType != null) { + app.adjType = adjType; app.adjTypeCode = ActivityManager.RunningAppProcessInfo .REASON_SERVICE_IN_USE; app.adjSource = cr.binding.client; @@ -13413,7 +13450,7 @@ public final class ActivityManagerService extends ActivityManagerNative // Ignore exceptions. } } - cleanUpApplicationRecordLocked(app, false, -1); + cleanUpApplicationRecordLocked(app, false, true, -1); mRemovedProcesses.remove(i); if (app.persistent) { diff --git a/services/java/com/android/server/am/ConnectionRecord.java b/services/java/com/android/server/am/ConnectionRecord.java index 22acda979856..01061147d673 100644 --- a/services/java/com/android/server/am/ConnectionRecord.java +++ b/services/java/com/android/server/am/ConnectionRecord.java @@ -32,6 +32,7 @@ class ConnectionRecord { final int clientLabel; // String resource labeling this client. final PendingIntent clientIntent; // How to launch the client. String stringName; // Caching of toString. + boolean serviceDead; // Well is it? void dump(PrintWriter pw, String prefix) { pw.println(prefix + "binding=" + binding); @@ -61,6 +62,9 @@ class ConnectionRecord { sb.append("ConnectionRecord{"); sb.append(Integer.toHexString(System.identityHashCode(this))); sb.append(' '); + if (serviceDead) { + sb.append("DEAD "); + } sb.append(binding.service.shortName); sb.append(":@"); sb.append(Integer.toHexString(System.identityHashCode(conn.asBinder()))); diff --git a/services/java/com/android/server/location/GeocoderProxy.java b/services/java/com/android/server/location/GeocoderProxy.java index e3131fe3ed85..b38ea13d5946 100644 --- a/services/java/com/android/server/location/GeocoderProxy.java +++ b/services/java/com/android/server/location/GeocoderProxy.java @@ -47,7 +47,9 @@ public class GeocoderProxy { public GeocoderProxy(Context context, String serviceName) { mContext = context; mIntent = new Intent(serviceName); - mContext.bindService(mIntent, mServiceConnection, Context.BIND_AUTO_CREATE); + mContext.bindService(mIntent, mServiceConnection, + Context.BIND_AUTO_CREATE | Context.BIND_NOT_FOREGROUND + | Context.BIND_ALLOW_OOM_MANAGEMENT); } /** @@ -58,7 +60,9 @@ public class GeocoderProxy { synchronized (mMutex) { mContext.unbindService(mServiceConnection); mServiceConnection = new Connection(); - mContext.bindService(mIntent, mServiceConnection, Context.BIND_AUTO_CREATE); + mContext.bindService(mIntent, mServiceConnection, + Context.BIND_AUTO_CREATE | Context.BIND_NOT_FOREGROUND + | Context.BIND_ALLOW_OOM_MANAGEMENT); } } @@ -67,14 +71,12 @@ public class GeocoderProxy { private IGeocodeProvider mProvider; public void onServiceConnected(ComponentName className, IBinder service) { - Log.d(TAG, "onServiceConnected " + className); synchronized (this) { mProvider = IGeocodeProvider.Stub.asInterface(service); } } public void onServiceDisconnected(ComponentName className) { - Log.d(TAG, "onServiceDisconnected " + className); synchronized (this) { mProvider = null; } diff --git a/services/java/com/android/server/location/LocationProviderProxy.java b/services/java/com/android/server/location/LocationProviderProxy.java index 1a1a17080d01..0bc166465bfa 100644 --- a/services/java/com/android/server/location/LocationProviderProxy.java +++ b/services/java/com/android/server/location/LocationProviderProxy.java @@ -28,7 +28,6 @@ import android.os.Bundle; import android.os.Handler; import android.os.IBinder; import android.os.RemoteException; -import android.os.SystemClock; import android.os.WorkSource; import android.util.Log; @@ -65,7 +64,9 @@ public class LocationProviderProxy implements LocationProviderInterface { mName = name; mIntent = new Intent(serviceName); mHandler = handler; - mContext.bindService(mIntent, mServiceConnection, Context.BIND_AUTO_CREATE); + mContext.bindService(mIntent, mServiceConnection, + Context.BIND_AUTO_CREATE | Context.BIND_NOT_FOREGROUND + | Context.BIND_ALLOW_OOM_MANAGEMENT); } /** @@ -76,7 +77,9 @@ public class LocationProviderProxy implements LocationProviderInterface { synchronized (mMutex) { mContext.unbindService(mServiceConnection); mServiceConnection = new Connection(); - mContext.bindService(mIntent, mServiceConnection, Context.BIND_AUTO_CREATE); + mContext.bindService(mIntent, mServiceConnection, + Context.BIND_AUTO_CREATE | Context.BIND_NOT_FOREGROUND + | Context.BIND_ALLOW_OOM_MANAGEMENT); } } @@ -88,7 +91,6 @@ public class LocationProviderProxy implements LocationProviderInterface { private DummyLocationProvider mCachedAttributes; // synchronized by mMutex public void onServiceConnected(ComponentName className, IBinder service) { - Log.d(TAG, "LocationProviderProxy.onServiceConnected " + className); synchronized (this) { mProvider = ILocationProvider.Stub.asInterface(service); if (mProvider != null) { @@ -98,7 +100,6 @@ public class LocationProviderProxy implements LocationProviderInterface { } public void onServiceDisconnected(ComponentName className) { - Log.d(TAG, "LocationProviderProxy.onServiceDisconnected " + className); synchronized (this) { mProvider = null; } -- cgit v1.2.3-59-g8ed1b