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

Commit b76aa50f authored by James Lemieux's avatar James Lemieux
Browse files

Fix alarm not firing in memory-pressure situations

Bug: 25846551

The original form of the code that fires an alarm is:
AlarmManager -> BroadcastReceiver -> Service.

The new form of the code that fires an alarm is:
AlarmManager -> Service.

Evidence exists that the system lowmemorykiller may elect to kill the
clock app after BroadcastReceiver.onReceive(...) completes but before
Service.onStartCommand(...) begins. When this occurs, the results are
disastrous as the clock fails to fire at the appropriate time.

To remove this possibility, all alarm state changes are delivered to the
Service. The methods that manipulate database state within
BroadcastReceiver have been made public and static and are called
directly from the Service to perform the same work as before. If the
alarm state transition is to the FIRING state, the AlarmService also
performs the work of posting the firing notification in the foreground.
All of this occurs during the handling of a single Intent on the same
thread within the service which should no longer provide
lowmemorykiller with any opportunities to prevent the firing of an alarm
by killing the clock app.

Change-Id: I3629a5b725a758f680f41611939a5bbeec23238a
parent 0dd0cac6
Loading
Loading
Loading
Loading
+8 −1
Original line number Diff line number Diff line
@@ -16,8 +16,10 @@

package com.android.deskclock;

import android.annotation.TargetApi;
import android.app.Activity;
import android.app.VoiceInteractor;
import android.os.Build;

/**
 * Notifies Voice Interactor about whether the action
@@ -34,6 +36,10 @@ public final class Voice {
        sDelegate = delegate;
    }

    public static Delegate getDelegate() {
        return sDelegate;
    }

    public static void notifySuccess(Activity activity, String message) {
        if (Utils.isMOrLater()) {
            sDelegate.notifySuccess(activity.getVoiceInteractor(), message);
@@ -52,6 +58,7 @@ public final class Voice {
        void notifyFailure(VoiceInteractor vi, String message);
    }

    @TargetApi(Build.VERSION_CODES.M)
    private static class VoiceInteractorDelegate implements Delegate {
        @Override
        public void notifySuccess(VoiceInteractor vi, String message) {
+7 −7
Original line number Diff line number Diff line
@@ -53,7 +53,7 @@ public final class AlarmNotifications {
        Intent hideIntent = AlarmStateManager.createStateChangeIntent(context,
                AlarmStateManager.ALARM_DELETE_TAG, instance,
                AlarmInstance.HIDE_NOTIFICATION_STATE);
        notification.setDeleteIntent(PendingIntent.getBroadcast(context, instance.hashCode(),
        notification.setDeleteIntent(PendingIntent.getService(context, instance.hashCode(),
                hideIntent, PendingIntent.FLAG_UPDATE_CURRENT));

        // Setup up dismiss action
@@ -61,7 +61,7 @@ public final class AlarmNotifications {
                AlarmStateManager.ALARM_DISMISS_TAG, instance, AlarmInstance.PREDISMISSED_STATE);
        notification.addAction(R.drawable.ic_alarm_off_24dp,
                context.getString(R.string.alarm_alert_dismiss_now_text),
                PendingIntent.getBroadcast(context, instance.hashCode(),
                PendingIntent.getService(context, instance.hashCode(),
                        dismissIntent, PendingIntent.FLAG_UPDATE_CURRENT));

        // Setup content action if instance is owned by alarm
@@ -95,7 +95,7 @@ public final class AlarmNotifications {
                AlarmStateManager.ALARM_DISMISS_TAG, instance, AlarmInstance.PREDISMISSED_STATE);
        notification.addAction(R.drawable.ic_alarm_off_24dp,
                context.getString(R.string.alarm_alert_dismiss_now_text),
                PendingIntent.getBroadcast(context, instance.hashCode(),
                PendingIntent.getService(context, instance.hashCode(),
                        dismissIntent, PendingIntent.FLAG_UPDATE_CURRENT));

        // Setup content action if instance is owned by alarm
@@ -127,7 +127,7 @@ public final class AlarmNotifications {
                AlarmStateManager.ALARM_DISMISS_TAG, instance, AlarmInstance.DISMISSED_STATE);
        notification.addAction(R.drawable.ic_alarm_off_24dp,
                context.getString(R.string.alarm_alert_dismiss_text),
                PendingIntent.getBroadcast(context, instance.hashCode(),
                PendingIntent.getService(context, instance.hashCode(),
                        dismissIntent, PendingIntent.FLAG_UPDATE_CURRENT));

        // Setup content action if instance is owned by alarm
@@ -159,7 +159,7 @@ public final class AlarmNotifications {
        // Setup dismiss intent
        Intent dismissIntent = AlarmStateManager.createStateChangeIntent(context,
                AlarmStateManager.ALARM_DISMISS_TAG, instance, AlarmInstance.DISMISSED_STATE);
        notification.setDeleteIntent(PendingIntent.getBroadcast(context, hashCode,
        notification.setDeleteIntent(PendingIntent.getService(context, hashCode,
                dismissIntent, PendingIntent.FLAG_UPDATE_CURRENT));

        // Setup content intent
@@ -194,7 +194,7 @@ public final class AlarmNotifications {
        Intent snoozeIntent = AlarmStateManager.createStateChangeIntent(service,
                AlarmStateManager.ALARM_SNOOZE_TAG, instance, AlarmInstance.SNOOZE_STATE);
        snoozeIntent.putExtra(AlarmStateManager.FROM_NOTIFICATION_EXTRA, true);
        PendingIntent snoozePendingIntent = PendingIntent.getBroadcast(service, instance.hashCode(),
        PendingIntent snoozePendingIntent = PendingIntent.getService(service, instance.hashCode(),
                snoozeIntent,
                PendingIntent.FLAG_UPDATE_CURRENT);
        notification.addAction(R.drawable.ic_snooze_24dp,
@@ -204,7 +204,7 @@ public final class AlarmNotifications {
        Intent dismissIntent = AlarmStateManager.createStateChangeIntent(service,
                AlarmStateManager.ALARM_DISMISS_TAG, instance, AlarmInstance.DISMISSED_STATE);
        dismissIntent.putExtra(AlarmStateManager.FROM_NOTIFICATION_EXTRA, true);
        PendingIntent dismissPendingIntent = PendingIntent.getBroadcast(service,
        PendingIntent dismissPendingIntent = PendingIntent.getService(service,
                instance.hashCode(), dismissIntent, PendingIntent.FLAG_UPDATE_CURRENT);
        notification.addAction(R.drawable.ic_alarm_off_24dp,
                resources.getString(R.string.alarm_alert_dismiss_text),
+21 −36
Original line number Diff line number Diff line
@@ -59,9 +59,6 @@ public class AlarmService extends Service {
    /** A public action sent by AlarmService when the alarm has stopped for any reason. */
    public static final String ALARM_DONE_ACTION = "com.android.deskclock.ALARM_DONE";

    /** Private action used to start an alarm with this service. */
    public static final String START_ALARM_ACTION = "START_ALARM";

    /** Private action used to stop an alarm with this service. */
    public static final String STOP_ALARM_ACTION = "STOP_ALARM";

@@ -86,22 +83,6 @@ public class AlarmService extends Service {
        return super.onUnbind(intent);
    }

    /**
     * Utility method to help start alarm properly. If alarm is already firing, it
     * will mark it as missed and start the new one.
     *
     * @param context application context
     * @param instance to trigger alarm
     */
    public static void startAlarm(Context context, AlarmInstance instance) {
        final Intent intent = AlarmInstance.createIntent(context, AlarmService.class, instance.mId)
                .setAction(START_ALARM_ACTION);

        // Maintain a cpu wake lock until the service can get it
        AlarmAlertWakeLock.acquireCpuWakeLock(context);
        context.startService(intent);
    }

    /**
     * Utility method to help stop an alarm properly. Nothing will happen, if alarm is not firing
     * or using a different instance.
@@ -129,7 +110,7 @@ public class AlarmService extends Service {
            // which kills the alarm. Check against the initial call state so
            // we don't kill the alarm during a call.
            if (state != TelephonyManager.CALL_STATE_IDLE && state != mInitialCallState) {
                sendBroadcast(AlarmStateManager.createStateChangeIntent(AlarmService.this,
                startService(AlarmStateManager.createStateChangeIntent(AlarmService.this,
                        "AlarmService", mCurrentAlarm, AlarmInstance.MISSED_STATE));
            }
        }
@@ -144,8 +125,6 @@ public class AlarmService extends Service {

        AlarmAlertWakeLock.acquireCpuWakeLock(this);

        Events.sendEvent(R.string.category_alarm, R.string.action_fire, 0);

        mCurrentAlarm = instance;
        AlarmNotifications.showAlarmNotification(this, mCurrentAlarm);
        mInitialCallState = mTelephonyManager.getCallState();
@@ -231,7 +210,12 @@ public class AlarmService extends Service {

        final long instanceId = AlarmInstance.getId(intent.getData());
        switch (intent.getAction()) {
            case START_ALARM_ACTION:
            case AlarmStateManager.CHANGE_STATE_ACTION:
                AlarmStateManager.handleIntent(this, intent);

                // If state is changed to firing, actually fire the alarm!
                final int alarmState = intent.getIntExtra(AlarmStateManager.ALARM_STATE_EXTRA, -1);
                if (alarmState == AlarmInstance.FIRED_STATE) {
                    final ContentResolver cr = this.getContentResolver();
                    final AlarmInstance instance = AlarmInstance.getInstance(cr, instanceId);
                    if (instance == null) {
@@ -248,6 +232,7 @@ public class AlarmService extends Service {
                        break;
                    }
                    startAlarm(instance);
                }
                break;
            case STOP_ALARM_ACTION:
                if (mCurrentAlarm != null && mCurrentAlarm.mId != instanceId) {
+12 −7
Original line number Diff line number Diff line
@@ -303,7 +303,13 @@ public final class AlarmStateManager extends BroadcastReceiver {
     */
    public static Intent createStateChangeIntent(Context context, String tag,
            AlarmInstance instance, Integer state) {
        Intent intent = AlarmInstance.createIntent(context, AlarmStateManager.class, instance.mId);
        // This intent is directed to AlarmService, though the actual handling of it occurs here
        // in AlarmStateManager. The reason is that evidence exists showing the jump between the
        // broadcast receiver (AlarmStateManager) and service (AlarmService) can be thwarted by the
        // Out Of Memory killer. If clock is killed during that jump, firing an alarm can fail to
        // occur. To be safer, the call begins in AlarmService, which has the power to display the
        // firing alarm if needed, so no jump is needed.
        Intent intent = AlarmInstance.createIntent(context, AlarmService.class, instance.mId);
        intent.setAction(CHANGE_STATE_ACTION);
        intent.addCategory(tag);
        intent.putExtra(ALARM_GLOBAL_ID_EXTRA, getGlobalIntentId(context));
@@ -448,8 +454,7 @@ public final class AlarmStateManager extends BroadcastReceiver {
                    instance.mId);
        }

        // Start the alarm and schedule timeout timer for it
        AlarmService.startAlarm(context, instance);
        Events.sendAlarmEvent(R.string.action_fire, 0);

        Calendar timeout = instance.getTimeout(context);
        if (timeout != null) {
@@ -829,7 +834,7 @@ public final class AlarmStateManager extends BroadcastReceiver {
     * @param instance to change state on
     * @param state to change to
     */
    public void setAlarmState(Context context, AlarmInstance instance, int state) {
    private static void setAlarmState(Context context, AlarmInstance instance, int state) {
        if (instance == null) {
            LogUtils.e("Null alarm instance while setting state to %d", state);
            return;
@@ -886,7 +891,7 @@ public final class AlarmStateManager extends BroadcastReceiver {
        });
    }

    private void handleIntent(Context context, Intent intent) {
    public static void handleIntent(Context context, Intent intent) {
        final String action = intent.getAction();
        LogUtils.v("AlarmStateManager received intent " + intent);
        if (CHANGE_STATE_ACTION.equals(action)) {
@@ -993,7 +998,7 @@ public final class AlarmStateManager extends BroadcastReceiver {
                    createStateChangeIntent(context, ALARM_MANAGER_TAG, instance, newState);
            // Treat alarm state change as high priority, use foreground broadcasts
            stateChangeIntent.addFlags(Intent.FLAG_RECEIVER_FOREGROUND);
            PendingIntent pendingIntent = PendingIntent.getBroadcast(context, instance.hashCode(),
            PendingIntent pendingIntent = PendingIntent.getService(context, instance.hashCode(),
                    stateChangeIntent, PendingIntent.FLAG_UPDATE_CURRENT);

            final AlarmManager am = (AlarmManager) context.getSystemService(Context.ALARM_SERVICE);
@@ -1005,7 +1010,7 @@ public final class AlarmStateManager extends BroadcastReceiver {
            LogUtils.v("Canceling instance " + instance.mId + " timers");

            // Create a PendingIntent that will match any one set for this instance
            PendingIntent pendingIntent = PendingIntent.getBroadcast(context, instance.hashCode(),
            PendingIntent pendingIntent = PendingIntent.getService(context, instance.hashCode(),
                    createStateChangeIntent(context, ALARM_MANAGER_TAG, instance, null),
                    PendingIntent.FLAG_NO_CREATE);

+1 −1
Original line number Diff line number Diff line
@@ -157,7 +157,7 @@ public final class AlarmTimeClickHandler {
        final Intent dismissIntent = AlarmStateManager.createStateChangeIntent(
                context, AlarmStateManager.ALARM_DISMISS_TAG, alarmInstance,
                AlarmInstance.PREDISMISSED_STATE);
        context.sendBroadcast(dismissIntent);
        context.startService(dismissIntent);
        mAlarmUpdateHandler.showPredismissToast(alarmInstance);
    }

Loading