Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 465fa396 authored by Dianne Hackborn's avatar Dianne Hackborn
Browse files

Fix issue #16907799: Processes containing bound services... 

...are killed over eagerly.

When the current foreground activity is moving to the background,
it was briefly going through the CACHED_ACTIVITY state before the
correct LAST_ACTIVITY state, allowing its bound service processes
to be killed (because they went in to the cached list).  To solve
this, as long as a process has stopping activities, it won't go
lower than LAST_ACTIVITY.

Also fixed a problem where we could put a process in CACHED_EMPTY
instead of CACHED_ACTIVITY_CLIENT.  There were a number of cases
in the binding flow and also the client process state transitions
where we would not correctly updateing the bound client activity
state.

And add some sanity code so that if a process hosting a
service is killed, and a client process of that service is in the
cached state, we kill the client process.  This avoids situations
where we can start thrashing around in the cached list because we
are restarting process for no reason -- since they will just
continue to be cached.

Finally, tune the process LRU list to allow twice as many cached
activity processes (from 8 to 16), so we can make better use of
the RAM we have available these days.

Change-Id: Ib0cdf78c321cbb035259fc9dd6ee27b5ba1f90c5
parent 4b5c2d3c

services/core/java/com/android/server/am/ActiveServices.java

+40 −4
Original line number Diff line number Diff line
@@ -29,6 +29,7 @@ import android.os.Handler; 
import android.os.Looper;

import android.os.SystemProperties;

import android.util.ArrayMap;

import android.util.ArraySet;

import com.android.internal.app.ProcessMap;

import com.android.internal.app.ProcessStats;

import com.android.internal.os.BatteryStatsImpl;
@@ -609,8 +610,25 @@ public final class ActiveServices { 
        mAm.updateProcessForegroundLocked(proc, anyForeground, oomAdj);

    }



    public void updateServiceConnectionActivitiesLocked(ProcessRecord clientProc) {

        ArraySet<ProcessRecord> updatedProcesses = null;

        for (int i=0; i<clientProc.connections.size(); i++) {

            final ConnectionRecord conn = clientProc.connections.valueAt(i);

            final ProcessRecord proc = conn.binding.service.app;

            if (proc == null || proc == clientProc) {

                continue;

            } else if (updatedProcesses == null) {

                updatedProcesses = new ArraySet<>();

            } else if (updatedProcesses.contains(proc)) {

                continue;

            }

            updatedProcesses.add(proc);

            updateServiceClientActivitiesLocked(proc, null, false);

        }

    }



    private boolean updateServiceClientActivitiesLocked(ProcessRecord proc,

            ConnectionRecord modCr) {

            ConnectionRecord modCr, boolean updateLru) {

        if (modCr != null && modCr.binding.client != null) {

            if (modCr.binding.client.activities.size() <= 0) {

                // This connection is from a client without activities, so adding
@@ -639,7 +657,9 @@ public final class ActiveServices { 
        }

        if (anyClientActivities != proc.hasClientActivities) {

            proc.hasClientActivities = anyClientActivities;

            if (updateLru) {

                mAm.updateLruProcessLocked(proc, anyClientActivities, null);

            }

            return true;

        }

        return false;
@@ -751,7 +771,7 @@ public final class ActiveServices { 
                b.client.hasAboveClient = true;

            }

            if (s.app != null) {

                updateServiceClientActivitiesLocked(s.app, c);

                updateServiceClientActivitiesLocked(s.app, c, true);

            }

            clist = mServiceConnections.get(binder);

            if (clist == null) {
@@ -1445,6 +1465,8 @@ public final class ActiveServices { 


        requestServiceBindingsLocked(r, execInFg);



        updateServiceClientActivitiesLocked(app, null, true);



        // If the service is in the started state, and there are no

        // pending arguments, then fake up one so its onStartCommand() will

        // be called.
@@ -1700,7 +1722,7 @@ public final class ActiveServices { 
                b.client.updateHasAboveClientLocked();

            }

            if (s.app != null) {

                updateServiceClientActivitiesLocked(s.app, c);

                updateServiceClientActivitiesLocked(s.app, c, true);

            }

        }

        clist = mServiceConnections.get(binder);
@@ -2077,6 +2099,19 @@ public final class ActiveServices { 
                        + ": shouldUnbind=" + b.hasBound);

                b.binder = null;

                b.requested = b.received = b.hasBound = false;

                // If this binding is coming from a cached process and is asking to keep

                // the service created, then we'll kill the cached process as well -- we

                // don't want to be thrashing around restarting processes that are only

                // there to be cached.

                for (int appi=b.apps.size()-1; appi>=0; appi--) {

                    ProcessRecord proc = b.apps.keyAt(appi);

                    if (proc != null && !proc.persistent && proc.thread != null

                            && proc.pid != 0 && proc.pid != ActivityManagerService.MY_PID

                            && proc.setProcState >= ActivityManager.PROCESS_STATE_LAST_ACTIVITY) {

                        proc.kill("bound to service " + sr.name.flattenToShortString()

                                + " in dying proc " + (app != null ? app.processName : "??"), true);

                    }

                }

            }

        }
@@ -2085,6 +2120,7 @@ public final class ActiveServices { 
            ConnectionRecord r = app.connections.valueAt(i);

            removeConnectionLocked(r, app, null);

        }

        updateServiceConnectionActivitiesLocked(app);

        app.connections.clear();



        ServiceMap smap = getServiceMap(app.userId);

services/core/java/com/android/server/am/ActivityManagerService.java

+9 −3
Original line number Diff line number Diff line
@@ -16404,8 +16404,8 @@ public final class ActivityManagerService extends ActivityManagerNative 
                    // send to process) since there can be an arbitrary number of stopping

                    // processes and they should soon all go into the cached state.

                    if (!r.finishing) {

                        if (procState > ActivityManager.PROCESS_STATE_CACHED_ACTIVITY) {

                            procState = ActivityManager.PROCESS_STATE_CACHED_ACTIVITY;

                        if (procState > ActivityManager.PROCESS_STATE_LAST_ACTIVITY) {

                            procState = ActivityManager.PROCESS_STATE_LAST_ACTIVITY;

                        }

                    }

                    app.cached = false;
@@ -17793,9 +17793,15 @@ public final class ActivityManagerService extends ActivityManagerNative 
        }















        if (DEBUG_OOM_ADJ) {













            if (false) {













                RuntimeException here = new RuntimeException("here");













                here.fillInStackTrace();













                Slog.d(TAG, "Did OOM ADJ in " + (SystemClock.uptimeMillis()-now) + "ms", here);













            } else {















                Slog.d(TAG, "Did OOM ADJ in " + (SystemClock.uptimeMillis()-now) + "ms");















            }















        }













    }





























    final void trimApplications() {















        synchronized (this) {

























services/core/java/com/android/server/am/ActivityStack.java



+3
−0




























Original line number
Diff line number
Diff line








@@ -3195,6 +3195,9 @@ final class ActivityStack {













                            ActivityManagerService.CANCEL_HEAVY_NOTIFICATION_MSG);















                }















                if (r.app.activities.isEmpty()) {













                    // Update any services we are bound to that might care about whether













                    // their client may have activities.













                    mService.mServices.updateServiceConnectionActivitiesLocked(r.app);















                    // No longer have activities, so update LRU list and oom adj.















                    mService.updateLruProcessLocked(r.app, false, null);















                    mService.updateOomAdjLocked();

































services/core/java/com/android/server/am/ActivityStackSupervisor.java



+4
−0




























Original line number
Diff line number
Diff line








@@ -1200,6 +1200,10 @@ public final class ActivityStackSupervisor implements DisplayListener {













            mService.startSetupActivityLocked();















        }





























        // Update any services we are bound to that might care about whether













        // their client may have activities.













        mService.mServices.updateServiceConnectionActivitiesLocked(r.app);





























        return true;















    }

















































services/core/java/com/android/server/am/ProcessList.java



+3
−3




























Original line number
Diff line number
Diff line








@@ -124,7 +124,7 @@ final class ProcessList {













    // we have no limit on the number of service, visible, foreground, or other such















    // processes and the number of those processes does not count against the cached















    // process limit.













    static final int MAX_CACHED_APPS = 24;













    static final int MAX_CACHED_APPS = 32;































    // We allow empty processes to stick around for at most 30 minutes.















    static final long MAX_EMPTY_TIME = 30*60*1000;









@@ -138,7 +138,7 @@ final class ProcessList {





























    // The number of cached at which we don't consider it necessary to do















    // memory trimming.













    static final int TRIM_CACHED_APPS = ((MAX_CACHED_APPS-MAX_EMPTY_APPS)*2)/3;













    static final int TRIM_CACHED_APPS = (MAX_CACHED_APPS-MAX_EMPTY_APPS)/3;































    // Threshold of number of cached+empty where we consider memory critical.















    static final int TRIM_CRITICAL_THRESHOLD = 3;










@@ -300,7 +300,7 @@ final class ProcessList {













    }































    public static int computeEmptyProcessLimit(int totalProcessLimit) {













        return (totalProcessLimit*2)/3;













        return totalProcessLimit/2;















    }































    private static String buildOomTag(String prefix, String space, int val, int base) {