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

Commit c8e11628 authored by Suprabh Shukla's avatar Suprabh Shukla
Browse files

Remove cancel listeners from pending intent alarms 

The cancel listeners are created per PendingIntent instance and were
spamming the callback list stored inside PendingIntentRecord. In cases
where there is even a single live PendingIntent backed by this
PendingIntentRecord, all PendingIntent instances backed by this
PendingIntentRecord for which a callback was ever registered will leak.

Test: atest FrameworksMockingServicesTests:\
com.android.server.am.PendingIntentControllerTest
atest FrameworksMockingServicesTests:\
com.android.server.AlarmManagerServiceTest

Bug: 143091024
Change-Id: I65df12da0c437064e6e3719911926738c677c4eb
Merged-In: I65df12da0c437064e6e3719911926738c677c4eb
(cherry picked from commit 0d51a8bc)
parent 903cd4ce

core/java/android/app/PendingIntent.java

+6 −1
Original line number Diff line number Diff line
@@ -1257,7 +1257,12 @@ public final class PendingIntent implements Parcelable { 
        return b != null ? new PendingIntent(b, in.getClassCookie(PendingIntent.class)) : null;

    }



    /*package*/ PendingIntent(IIntentSender target) {

    /**

     * Creates a PendingIntent with the given target.

     * @param target the backing IIntentSender

     * @hide

     */

    public PendingIntent(IIntentSender target) {

        mTarget = target;

    }

services/core/java/com/android/server/AlarmManagerInternal.java

+8 −0
Original line number Diff line number Diff line
@@ -16,6 +16,8 @@ 


package com.android.server;



import android.app.PendingIntent;



public interface AlarmManagerInternal {

    // Some other components in the system server need to know about

    // broadcast alarms currently in flight
@@ -30,4 +32,10 @@ public interface AlarmManagerInternal { 
    boolean isIdling();

    public void removeAlarmsForUid(int uid);

    public void registerInFlightListener(InFlightListener callback);



    /**

     * Removes any alarm with the given pending intent with equality determined using

     * {@link android.app.PendingIntent#equals(java.lang.Object) PendingIntent.equals}

     */

    void remove(PendingIntent rec);

}

services/core/java/com/android/server/AlarmManagerService.java

+10 −21
Original line number Diff line number Diff line
@@ -210,7 +210,6 @@ class AlarmManagerService extends SystemService { 
    IAlarmListener mTimeTickTrigger;

    PendingIntent mDateChangeSender;

    Random mRandom;

    PendingIntent.CancelListener mOperationCancelListener;

    boolean mInteractive = true;

    long mNonInteractiveStartTime;

    long mNonInteractiveTime;
@@ -1498,7 +1497,6 @@ class AlarmManagerService extends SystemService { 


        synchronized (mLock) {

            mHandler = new AlarmHandler();

            mOperationCancelListener = (intent) -> removeImpl(intent, null);

            mConstants = new Constants(mHandler);

            mAppWakeupHistory = new AppWakeupHistory(Constants.DEFAULT_APP_STANDBY_WINDOW);
@@ -1750,9 +1748,6 @@ class AlarmManagerService extends SystemService { 
        } else {

            maxElapsed = triggerElapsed + windowLength;

        }

        if (operation != null) {

            operation.registerCancelListener(mOperationCancelListener);

        }

        synchronized (mLock) {

            if (DEBUG_BATCH) {

                Slog.v(TAG, "set(" + operation + ") : type=" + type
@@ -1765,8 +1760,6 @@ class AlarmManagerService extends SystemService { 
                        "Maximum limit of concurrent alarms " + mConstants.MAX_ALARMS_PER_UID

                                + " reached for uid: " + UserHandle.formatUid(callingUid)

                                + ", callingPackage: " + callingPackage;

                mHandler.obtainMessage(AlarmHandler.UNREGISTER_CANCEL_LISTENER,

                        operation).sendToTarget();

                Slog.w(TAG, errorMsg);

                throw new IllegalStateException(errorMsg);

            }
@@ -1787,8 +1780,6 @@ class AlarmManagerService extends SystemService { 
            if (ActivityManager.getService().isAppStartModeDisabled(callingUid, callingPackage)) {

                Slog.w(TAG, "Not setting alarm from " + callingUid + ":" + a

                        + " -- package not allowed to start");

                mHandler.obtainMessage(AlarmHandler.UNREGISTER_CANCEL_LISTENER,

                        operation).sendToTarget();

                return;

            }

        } catch (RemoteException e) {
@@ -2041,6 +2032,11 @@ class AlarmManagerService extends SystemService { 
            }

        }



        @Override

        public void remove(PendingIntent pi) {

            mHandler.obtainMessage(AlarmHandler.REMOVE_FOR_CANCELED, pi).sendToTarget();

        }



        @Override

        public void registerInFlightListener(InFlightListener callback) {

            synchronized (mLock) {
@@ -2146,8 +2142,6 @@ class AlarmManagerService extends SystemService { 
            synchronized (mLock) {

                removeLocked(operation, listener);

            }

            mHandler.obtainMessage(AlarmHandler.UNREGISTER_CANCEL_LISTENER,

                    operation).sendToTarget();

        }



        @Override
@@ -4151,7 +4145,7 @@ class AlarmManagerService extends SystemService { 
        public static final int APP_STANDBY_BUCKET_CHANGED = 5;

        public static final int APP_STANDBY_PAROLE_CHANGED = 6;

        public static final int REMOVE_FOR_STOPPED = 7;

        public static final int UNREGISTER_CANCEL_LISTENER = 8;

        public static final int REMOVE_FOR_CANCELED = 8;



        AlarmHandler() {

            super(Looper.myLooper());
@@ -4234,10 +4228,10 @@ class AlarmManagerService extends SystemService { 
                    }

                    break;



                case UNREGISTER_CANCEL_LISTENER:

                    final PendingIntent pi = (PendingIntent) msg.obj;

                    if (pi != null) {

                        pi.unregisterCancelListener(mOperationCancelListener);

                case REMOVE_FOR_CANCELED:

                    final PendingIntent operation = (PendingIntent) msg.obj;

                    synchronized (mLock) {

                        removeLocked(operation, null);

                    }

                    break;
@@ -4696,11 +4690,6 @@ class AlarmManagerService extends SystemService { 
                                        Intent.EXTRA_ALARM_COUNT, alarm.count),

                                mDeliveryTracker, mHandler, null,

                                allowWhileIdle ? mIdleOptions : null);

                        if (alarm.repeatInterval == 0) {

                            // Keep the listener for repeating alarms until they get cancelled

                            mHandler.obtainMessage(AlarmHandler.UNREGISTER_CANCEL_LISTENER,

                                    alarm.operation).sendToTarget();

                        }

                    } catch (PendingIntent.CanceledException e) {

                        if (alarm.repeatInterval > 0) {

                            // This IntentSender is no longer valid, but this

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

+3 −0
Original line number Diff line number Diff line
@@ -43,6 +43,7 @@ import android.util.Slog; 


import com.android.internal.os.IResultReceiver;

import com.android.internal.util.function.pooled.PooledLambda;

import com.android.server.AlarmManagerInternal;

import com.android.server.LocalServices;

import com.android.server.wm.ActivityTaskManagerInternal;

import com.android.server.wm.SafeActivityOptions;
@@ -293,6 +294,8 @@ public class PendingIntentController { 
                    PendingIntentController::handlePendingIntentCancelled, this, callbacks);

            mH.sendMessage(m);

        }

        final AlarmManagerInternal ami = LocalServices.getService(AlarmManagerInternal.class);

        ami.remove(new PendingIntent(rec));

    }



    private void handlePendingIntentCancelled(RemoteCallbackList<IResultReceiver> callbacks) {

services/tests/mockingservicestests/src/com/android/server/AlarmManagerServiceTest.java

+18 −14
Original line number Diff line number Diff line
@@ -24,6 +24,7 @@ import static android.app.usage.UsageStatsManager.STANDBY_BUCKET_FREQUENT; 
import static android.app.usage.UsageStatsManager.STANDBY_BUCKET_RARE;

import static android.app.usage.UsageStatsManager.STANDBY_BUCKET_WORKING_SET;



import static com.android.dx.mockito.inline.extended.ExtendedMockito.doCallRealMethod;

import static com.android.dx.mockito.inline.extended.ExtendedMockito.doNothing;

import static com.android.dx.mockito.inline.extended.ExtendedMockito.doReturn;

import static com.android.dx.mockito.inline.extended.ExtendedMockito.doThrow;
@@ -35,6 +36,7 @@ import static com.android.dx.mockito.inline.extended.ExtendedMockito.when; 
import static com.android.server.AlarmManagerService.ACTIVE_INDEX;

import static com.android.server.AlarmManagerService.AlarmHandler.APP_STANDBY_BUCKET_CHANGED;

import static com.android.server.AlarmManagerService.AlarmHandler.APP_STANDBY_PAROLE_CHANGED;

import static com.android.server.AlarmManagerService.AlarmHandler.REMOVE_FOR_CANCELED;

import static com.android.server.AlarmManagerService.Constants.KEY_ALLOW_WHILE_IDLE_LONG_TIME;

import static com.android.server.AlarmManagerService.Constants.KEY_ALLOW_WHILE_IDLE_SHORT_TIME;

import static com.android.server.AlarmManagerService.Constants.KEY_ALLOW_WHILE_IDLE_WHITELIST_DURATION;
@@ -79,9 +81,9 @@ import android.provider.Settings; 
import android.util.Log;

import android.util.SparseArray;



import androidx.test.filters.FlakyTest;

import androidx.test.runner.AndroidJUnit4;



import com.android.dx.mockito.inline.extended.MockedVoidMethod;

import com.android.internal.annotations.GuardedBy;



import org.junit.After;
@@ -166,7 +168,6 @@ public class AlarmManagerServiceTest { 
    }



    public class Injector extends AlarmManagerService.Injector {

        boolean mIsAutomotiveOverride;



        Injector(Context context) {

            super(context);
@@ -256,6 +257,9 @@ public class AlarmManagerServiceTest { 
                .when(() -> LocalServices.getService(DeviceIdleController.LocalService.class));

        doReturn(mUsageStatsManagerInternal).when(

                () -> LocalServices.getService(UsageStatsManagerInternal.class));

        doCallRealMethod().when((MockedVoidMethod) () ->

                LocalServices.addService(eq(AlarmManagerInternal.class), any()));

        doCallRealMethod().when(() -> LocalServices.getService(AlarmManagerInternal.class));

        when(mUsageStatsManagerInternal.getAppStandbyBucket(eq(TEST_CALLING_PACKAGE),

                eq(UserHandle.getUserId(TEST_CALLING_UID)), anyLong()))

                .thenReturn(STANDBY_BUCKET_ACTIVE);
@@ -443,7 +447,6 @@ public class AlarmManagerServiceTest { 
        assertEquals(expectedTriggerTime, mTestTimer.getElapsed());

    }



    @FlakyTest(bugId = 130313408)

    @Test

    public void testEarliestAlarmSet() {

        final PendingIntent pi6 = getNewMockPendingIntent();
@@ -661,11 +664,15 @@ public class AlarmManagerServiceTest { 
                anyLong())).thenReturn(bucket);

        mAppStandbyListener.onAppIdleStateChanged(TEST_CALLING_PACKAGE,

                UserHandle.getUserId(TEST_CALLING_UID), false, bucket, 0);

        assertAndHandleMessageSync(APP_STANDBY_BUCKET_CHANGED);

    }



    private void assertAndHandleMessageSync(int what) {

        final ArgumentCaptor<Message> messageCaptor = ArgumentCaptor.forClass(Message.class);

        verify(mService.mHandler, atLeastOnce()).sendMessage(messageCaptor.capture());

        final Message lastMessage = messageCaptor.getValue();

        assertEquals("Unexpected message send to handler", lastMessage.what,

                APP_STANDBY_BUCKET_CHANGED);

                what);

        mService.mHandler.handleMessage(lastMessage);

    }
@@ -725,12 +732,7 @@ public class AlarmManagerServiceTest { 


    private void assertAndHandleParoleChanged(boolean parole) {

        mAppStandbyListener.onParoleStateChanged(parole);

        final ArgumentCaptor<Message> messageCaptor = ArgumentCaptor.forClass(Message.class);

        verify(mService.mHandler, atLeastOnce()).sendMessage(messageCaptor.capture());

        final Message lastMessage = messageCaptor.getValue();

        assertEquals("Unexpected message send to handler", lastMessage.what,

                APP_STANDBY_PAROLE_CHANGED);

        mService.mHandler.handleMessage(lastMessage);

        assertAndHandleMessageSync(APP_STANDBY_PAROLE_CHANGED);

    }



    @Test
@@ -1033,12 +1035,13 @@ public class AlarmManagerServiceTest { 
    }



    @Test

    public void alarmCountOnPendingIntentCancel() {

    public void alarmCountOnRemoveForCanceled() {

        final AlarmManagerInternal ami = LocalServices.getService(AlarmManagerInternal.class);

        final PendingIntent pi = getNewMockPendingIntent();

        setTestAlarm(ELAPSED_REALTIME_WAKEUP, mNowElapsedTest + 123, pi);

        verify(pi).registerCancelListener(mService.mOperationCancelListener);

        setTestAlarm(ELAPSED_REALTIME, mNowElapsedTest + 12345, pi);

        assertEquals(1, mService.mAlarmsPerUid.get(TEST_CALLING_UID));

        mService.mOperationCancelListener.onCancelled(pi);

        ami.remove(pi);

        assertAndHandleMessageSync(REMOVE_FOR_CANCELED);

        assertEquals(0, mService.mAlarmsPerUid.get(TEST_CALLING_UID));

    }
@@ -1047,5 +1050,6 @@ public class AlarmManagerServiceTest { 
        if (mMockingSession != null) {

            mMockingSession.finishMocking();

        }

        LocalServices.removeServiceForTest(AlarmManagerInternal.class);

    }

}