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

Commit f82236ef authored by Dianne Hackborn's avatar Dianne Hackborn
Browse files

Fix issue #37306208: CTS: ServiceTest#testForegroundService... 

..._removeNotificationOnStop failure

We weren't removing the foreground timeout when a service is
brought down and still waiting for the app to make it foreground.
Instead, turn this in to an immediate crash of the app when we
detect it while bringing down the service.  Also make various
paths more robust against trying to do things with a ServiceRecord
that isn't currently in ActiveServices.

Test: bit CtsAppTestCases:ServiceTest

Change-Id: I06f4ac48fb983483569677ab1ac6e5b0b681c4ae
parent 6e8f1166

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

+49 −9
Original line number Diff line number Diff line
@@ -761,7 +761,7 @@ public final class ActiveServices { 
                }

            }

            if (r.fgRequired) {

                if (DEBUG_BACKGROUND_CHECK) {

                if (DEBUG_SERVICE || DEBUG_BACKGROUND_CHECK) {

                    Slog.i(TAG, "Service called startForeground() as required: " + r);

                }

                r.fgRequired = false;
@@ -1339,16 +1339,19 @@ public final class ActiveServices { 
        final ComponentName comp = service.getComponent();

        if (comp != null) {

            r = smap.mServicesByName.get(comp);

            if (DEBUG_SERVICE && r != null) Slog.v(TAG_SERVICE, "Retrieved by component: " + r);

        }

        if (r == null && !isBindExternal) {

            Intent.FilterComparison filter = new Intent.FilterComparison(service);

            r = smap.mServicesByIntent.get(filter);

            if (DEBUG_SERVICE && r != null) Slog.v(TAG_SERVICE, "Retrieved by intent: " + r);

        }

        if (r != null && (r.serviceInfo.flags & ServiceInfo.FLAG_EXTERNAL_SERVICE) != 0

                && !callingPackage.equals(r.packageName)) {

            // If an external service is running within its own package, other packages

            // should not bind to that instance.

            r = null;

            if (DEBUG_SERVICE) Slog.v(TAG_SERVICE, "Whoops, can't use existing external service");

        }

        if (r == null) {

            try {
@@ -1408,12 +1411,14 @@ public final class ActiveServices { 
                    sInfo.applicationInfo = mAm.getAppInfoForUser(sInfo.applicationInfo, userId);

                }

                r = smap.mServicesByName.get(name);

                if (DEBUG_SERVICE && r != null) Slog.v(TAG_SERVICE,

                        "Retrieved via pm by intent: " + r);

                if (r == null && createIfNeeded) {

                    Intent.FilterComparison filter

                    final Intent.FilterComparison filter

                            = new Intent.FilterComparison(service.cloneFilter());

                    ServiceRestarter res = new ServiceRestarter();

                    BatteryStatsImpl.Uid.Pkg.Serv ss = null;

                    BatteryStatsImpl stats = mAm.mBatteryStatsService.getActiveStatistics();

                    final ServiceRestarter res = new ServiceRestarter();

                    final BatteryStatsImpl.Uid.Pkg.Serv ss;

                    final BatteryStatsImpl stats = mAm.mBatteryStatsService.getActiveStatistics();

                    synchronized (stats) {

                        ss = stats.getServiceStatsLocked(

                                sInfo.applicationInfo.uid, sInfo.packageName,
@@ -1426,12 +1431,14 @@ public final class ActiveServices { 


                    // Make sure this component isn't in the pending list.

                    for (int i=mPendingServices.size()-1; i>=0; i--) {

                        ServiceRecord pr = mPendingServices.get(i);

                        final ServiceRecord pr = mPendingServices.get(i);

                        if (pr.serviceInfo.applicationInfo.uid == sInfo.applicationInfo.uid

                                && pr.name.equals(name)) {

                            if (DEBUG_SERVICE) Slog.v(TAG_SERVICE, "Remove pending: " + pr);

                            mPendingServices.remove(i);

                        }

                    }

                    if (DEBUG_SERVICE) Slog.v(TAG_SERVICE, "Retrieve created new service: " + r);

                }

            } catch (RemoteException ex) {

                // pm is in same process, this will never happen.
@@ -2119,7 +2126,28 @@ public final class ActiveServices { 
            }

        }



        if (DEBUG_SERVICE) Slog.v(TAG_SERVICE, "Bringing down " + r + " " + r.intent);

        // Check to see if the service had been started as foreground, but being

        // brought down before actually showing a notification.  That is not allowed.

        if (r.fgRequired) {

            Slog.w(TAG_SERVICE, "Bringing down service while still waiting for start foreground: "

                    + r);

            r.fgRequired = false;

            r.fgWaiting = false;

            mAm.mHandler.removeMessages(

                    ActivityManagerService.SERVICE_FOREGROUND_TIMEOUT_MSG, r);

            if (r.app != null) {

                Message msg = mAm.mHandler.obtainMessage(

                        ActivityManagerService.SERVICE_FOREGROUND_CRASH_MSG);

                msg.obj = r.app;

                mAm.mHandler.sendMessage(msg);

            }

        }



        if (DEBUG_SERVICE) {

            RuntimeException here = new RuntimeException();

            here.fillInStackTrace();

            Slog.v(TAG_SERVICE, "Bringing down " + r + " " + r.intent, here);

        }

        r.destroyTime = SystemClock.uptimeMillis();

        if (LOG_SERVICE_START_STOP) {

            EventLogTags.writeAmDestroyService(
@@ -2127,7 +2155,14 @@ public final class ActiveServices { 
        }



        final ServiceMap smap = getServiceMapLocked(r.userId);

        smap.mServicesByName.remove(r.name);

        ServiceRecord found = smap.mServicesByName.remove(r.name);

        if (found != r) {

            // This is not actually the service we think is running...  this should not happen,

            // but if it does, fail hard.

            smap.mServicesByName.put(r.name, found);

            throw new IllegalStateException("Bringing down " + r + " but actually running "

                    + found);

        }

        smap.mServicesByIntent.remove(r.intent);

        r.totalRestartCount = 0;

        unscheduleServiceRestartLocked(r, 0, true);
@@ -2967,7 +3002,7 @@ public final class ActiveServices { 
    void serviceForegroundTimeout(ServiceRecord r) {

        ProcessRecord app;

        synchronized (mAm) {

            if (!r.fgRequired) {

            if (!r.fgRequired || r.destroying) {

                return;

            }
@@ -2985,6 +3020,11 @@ public final class ActiveServices { 
        }

    }



    void serviceForegroundCrash(ProcessRecord app) {

        mAm.crashApplication(app.uid, app.pid, app.info.packageName, app.userId,

                "Context.startForegroundService() did not then call Service.startForeground()");

    }



    void scheduleServiceTimeoutLocked(ProcessRecord proc) {

        if (proc.executingServices.size() == 0 || proc.thread == null) {

            return;

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

+4 −0
Original line number Diff line number Diff line
@@ -1703,6 +1703,7 @@ public class ActivityManagerService extends IActivityManager.Stub 
    static final int SERVICE_FOREGROUND_TIMEOUT_MSG = 66;

    static final int DISPATCH_PENDING_INTENT_CANCEL_MSG = 67;

    static final int PUSH_TEMP_WHITELIST_UI_MSG = 68;

    static final int SERVICE_FOREGROUND_CRASH_MSG = 69;

    static final int START_USER_SWITCH_FG_MSG = 712;















    static final int FIRST_ACTIVITY_STACK_MSG = 100;










@@ -1974,6 +1975,9 @@ public class ActivityManagerService extends IActivityManager.Stub













            case SERVICE_FOREGROUND_TIMEOUT_MSG: {















                mServices.serviceForegroundTimeout((ServiceRecord)msg.obj);















            } break;













            case SERVICE_FOREGROUND_CRASH_MSG: {













                mServices.serviceForegroundCrash((ProcessRecord)msg.obj);













            } break;















            case DISPATCH_PENDING_INTENT_CANCEL_MSG: {















                RemoteCallbackList<IResultReceiver> callbacks















                        = (RemoteCallbackList<IResultReceiver>)msg.obj;