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

Commit 7d40780e authored by Suprabh Shukla's avatar Suprabh Shukla
Browse files

Don't remove OnAlarmListener mappings in the client 

Removing the mapping is prone to races in keeping the state consistent
with the server. An inconsistent state can lead to multiple alarms on
the server for the same listener.

Test: atest CtsJobSchedulerTestCases doesn't fill the alarm queue with
more than one alarm.
atest CtsAlarmManagerTestCases

Bug: 154444435
Change-Id: Iaf11d5decb17fbf2366b49d9865231bf65dbdc41
parent 8a36b147

core/java/android/app/AlarmManager.java

+20 −22
Original line number Diff line number Diff line
@@ -31,7 +31,6 @@ import android.os.Parcelable; 
import android.os.RemoteException;

import android.os.WorkSource;

import android.text.TextUtils;

import android.util.ArrayMap;

import android.util.Log;

import android.util.proto.ProtoOutputStream;
@@ -40,6 +39,8 @@ import libcore.timezone.ZoneInfoDb; 
import java.io.IOException;

import java.lang.annotation.Retention;

import java.lang.annotation.RetentionPolicy;

import java.lang.ref.WeakReference;

import java.util.WeakHashMap;



/**

 * This class provides access to the system alarm services.  These allow you
@@ -222,26 +223,12 @@ public class AlarmManager { 
            } catch (RemoteException ex) {

                throw ex.rethrowFromSystemServer();

            }



            synchronized (AlarmManager.class) {

                if (sWrappers != null) {

                    sWrappers.remove(mListener);

                }

            }

        }



        @Override

        public void doAlarm(IAlarmCompleteListener alarmManager) {

            mCompletion = alarmManager;



            // Remove this listener from the wrapper cache first; the server side

            // already considers it gone

            synchronized (AlarmManager.class) {

                if (sWrappers != null) {

                    sWrappers.remove(mListener);

                }

            }



            mHandler.post(this);

        }
@@ -263,9 +250,14 @@ public class AlarmManager { 
        }

    }



    // Tracking of the OnAlarmListener -> wrapper mapping, for cancel() support.

    // Access is synchronized on the AlarmManager class object.

    private static ArrayMap<OnAlarmListener, ListenerWrapper> sWrappers;

    /**

     * Tracking of the OnAlarmListener -> ListenerWrapper mapping, for cancel() support.

     * An entry is guaranteed to stay in this map as long as its ListenerWrapper is held by the

     * server.

     *

     * <p>Access is synchronized on the AlarmManager class object.

     */

    private static WeakHashMap<OnAlarmListener, WeakReference<ListenerWrapper>> sWrappers;



    /**

     * package private on purpose
@@ -682,14 +674,17 @@ public class AlarmManager { 
        if (listener != null) {

            synchronized (AlarmManager.class) {

                if (sWrappers == null) {

                    sWrappers = new ArrayMap<OnAlarmListener, ListenerWrapper>();

                    sWrappers = new WeakHashMap<>();

                }



                recipientWrapper = sWrappers.get(listener);

                final WeakReference<ListenerWrapper> weakRef = sWrappers.get(listener);

                if (weakRef != null) {

                    recipientWrapper = weakRef.get();

                }

                // no existing wrapper => build a new one

                if (recipientWrapper == null) {

                    recipientWrapper = new ListenerWrapper(listener);

                    sWrappers.put(listener, recipientWrapper);

                    sWrappers.put(listener, new WeakReference<>(recipientWrapper));

                }

            }
@@ -948,7 +943,10 @@ public class AlarmManager { 
        ListenerWrapper wrapper = null;

        synchronized (AlarmManager.class) {

            if (sWrappers != null) {

                wrapper = sWrappers.get(listener);

                final WeakReference<ListenerWrapper> weakRef = sWrappers.get(listener);

                if (weakRef != null) {

                    wrapper = weakRef.get();

                }

            }

        }