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

Commit a18b884e authored by Matías Hernández's avatar Matías Hernández
Browse files

Fix rate-limiting of enqueued notifications 

NMS has a rate-limiting mechanism for notification enqueues. In theory, this should stop packages from enqueueing updates (only updates) to notifications at a rate of more than 5/sec.

Also, when a notification is enqueued, NMS waits 250ms for the NAS to do its magic and calculate appropriate adjustments before the notification is actually posted.

This led to some issues with enqueue rate-limiting -- a rogue or malfunctioning app could enqueue as many notification updates are they could fit in this 250ms window, because:
1. Only updates to _already-posted_ notifications were considered updates.
2. The enqueue rate was updated at post time, not at enqueueing time.
3. The calculated enqueue rate catches up very slowly to the actual rate for very "bursty" events.

This CL fixes these issues:
1. Updates to _still-queued_ notifications are also considered updates.
2. The enqueue rate is updated _at enqueue time_, but _only for notifications that get through_ the rate-limiting. This hews closer to the original implementation, and the intent is that packages that post at a constant above the rate-limit (say 8/sec) should still get _some_ notifications through (around half in this case) instead of all of them being blocked.
3. The alpha value of the RateEstimator is tweaked to give more weight to the latest events. Otherwise an app can post one update, then sit idle for a long time, then post a heavy burst, and the estimated rate would take a long time to catch up to the new behavior. This still doesn't mean that a package that posts 6 updates will see the sixth blocked, but the rate calculation is now more sensitive.

Test: Unit tests + manual
Fixes: 280098361
Change-Id: I08568ce90427f18a9b23530cd7b6cbf45b9d9395
parent a73b99ad

services/core/java/com/android/server/notification/NotificationManagerService.java

+18 −46
Original line number Diff line number Diff line
@@ -6746,6 +6746,8 @@ public class NotificationManagerService extends SystemService { 
            return false;

        }













        mUsageStats.registerEnqueuedByAppAndAccepted(pkg);



























        if (info != null) {















            // Cache the shortcut synchronously after the associated notification is posted in case















            // the app unpublishes this shortcut immediately after posting the notification. If the










@@ -7184,11 +7186,12 @@ public class NotificationManagerService extends SystemService {













                            + " cannot create notifications");















                }



























                // rate limit updates that aren't completed progress notifications













                if (mNotificationsByKey.get(r.getSbn().getKey()) != null













                        && !r.getNotification().hasCompletedProgress()













                        && !isAutogroup) {

























                // Rate limit updates that aren't completed progress notifications













                // Search for the original one in the posted and not-yet-posted (enqueued) lists.













                boolean isUpdate = mNotificationsByKey.get(r.getSbn().getKey()) != null













                        || findNotificationByListLocked(mEnqueuedNotifications, r.getSbn().getKey())













                        != null;













                if (isUpdate && !r.getNotification().hasCompletedProgress() && !isAutogroup) {















                    final float appEnqueueRate = mUsageStats.getAppEnqueueRate(pkg);















                    if (appEnqueueRate > mMaxPackageEnqueueRate) {















                        mUsageStats.registerOverRateQuota(pkg);










@@ -7813,15 +7816,8 @@ public class NotificationManagerService extends SystemService {













            boolean posted = false;















            synchronized (mNotificationLock) {















                try {













                    NotificationRecord r = null;













                    int N = mEnqueuedNotifications.size();













                    for (int i = 0; i < N; i++) {













                        final NotificationRecord enqueued = mEnqueuedNotifications.get(i);













                        if (Objects.equals(key, enqueued.getKey())) {













                            r = enqueued;













                            break;













                        }













                    }













                    NotificationRecord r = findNotificationByListLocked(mEnqueuedNotifications,













                            key);















                    if (r == null) {















                        Slog.i(TAG, "Cannot find enqueued record for key: " + key);















                        return false;










@@ -9486,7 +9482,7 @@ public class NotificationManagerService extends SystemService {













     * Determine whether the userId applies to the notification in question, either because















     * they match exactly, or one of them is USER_ALL (which is treated as a wildcard).















     */













    private boolean notificationMatchesUserId(NotificationRecord r, int userId) {













    private static boolean notificationMatchesUserId(NotificationRecord r, int userId) {















        return















                // looking for USER_ALL notifications? match everything















                   userId == UserHandle.USER_ALL










@@ -9845,9 +9841,9 @@ public class NotificationManagerService extends SystemService {













        return null;















    }



























    @GuardedBy("mNotificationLock")













    private NotificationRecord findNotificationByListLocked(ArrayList<NotificationRecord> list,













            String pkg, String tag, int id, int userId) {













    @Nullable













    private static NotificationRecord findNotificationByListLocked(













            ArrayList<NotificationRecord> list, String pkg, String tag, int id, int userId) {















        final int len = list.size();















        for (int i = 0; i < len; i++) {















            NotificationRecord r = list.get(i);









@@ -9860,8 +9856,7 @@ public class NotificationManagerService extends SystemService {













        return null;















    }



























    @GuardedBy("mNotificationLock")













    private List<NotificationRecord> findNotificationsByListLocked(













    private static List<NotificationRecord> findNotificationsByListLocked(















            ArrayList<NotificationRecord> list, String pkg, String tag, int id, int userId) {















        List<NotificationRecord> matching = new ArrayList<>();















        final int len = list.size();









@@ -9876,9 +9871,9 @@ public class NotificationManagerService extends SystemService {













        return matching;















    }



























    @GuardedBy("mNotificationLock")













    private NotificationRecord findNotificationByListLocked(ArrayList<NotificationRecord> list,













            String key) {













    @Nullable













    private static NotificationRecord findNotificationByListLocked(













            ArrayList<NotificationRecord> list, String key) {















        final int N = list.size();















        for (int i = 0; i < N; i++) {















            if (key.equals(list.get(i).getKey())) {









@@ -9888,29 +9883,6 @@ public class NotificationManagerService extends SystemService {













        return null;















    }



























    /**













     * There may be multiple records that match your criteria. For instance if there have been













     * multiple notifications posted which are enqueued for the same pkg, tag, id, userId. This













     * method will find all of them in the given list













     * @return













     */













    @GuardedBy("mNotificationLock")













    private List<NotificationRecord> findEnqueuedNotificationsForCriteria(













            String pkg, String tag, int id, int userId) {













        final ArrayList<NotificationRecord> records = new ArrayList<>();













        final int n = mEnqueuedNotifications.size();













        for (int i = 0; i < n; i++) {













            NotificationRecord r = mEnqueuedNotifications.get(i);













            if (notificationMatchesUserId(r, userId)













                    && r.getSbn().getId() == id













                    && TextUtils.equals(r.getSbn().getTag(), tag)













                    && r.getSbn().getPackageName().equals(pkg)) {













                records.add(r);













            }













        }













        return records;













    }



























    @GuardedBy("mNotificationLock")















    int indexOfNotificationLocked(String key) {















        final int N = mNotificationList.size();

































services/core/java/com/android/server/notification/NotificationUsageStats.java



+30
−30




























Original line number
Diff line number
Diff line








@@ -16,23 +16,16 @@





























package com.android.server.notification;





























import static android.app.NotificationManager.IMPORTANCE_HIGH;





























import android.app.Notification;













import android.content.ContentValues;















import android.content.Context;













import android.database.Cursor;













import android.database.sqlite.SQLiteDatabase;













import android.database.sqlite.SQLiteFullException;













import android.database.sqlite.SQLiteOpenHelper;















import android.os.Handler;













import android.os.HandlerThread;















import android.os.Message;















import android.os.SystemClock;















import android.text.TextUtils;















import android.util.ArraySet;















import android.util.Log;





























import com.android.internal.annotations.GuardedBy;















import com.android.internal.logging.MetricsLogger;















import com.android.server.notification.NotificationManagerService.DumpFilter;

























@@ -42,8 +35,6 @@ import org.json.JSONObject;





























import java.io.PrintWriter;















import java.util.ArrayDeque;













import java.util.Calendar;













import java.util.GregorianCalendar;















import java.util.HashMap;















import java.util.Map;















import java.util.Set;









@@ -63,7 +54,6 @@ public class NotificationUsageStats {













    private static final String TAG = "NotificationUsageStats";































    private static final boolean ENABLE_AGGREGATED_IN_MEMORY_STATS = true;













    private static final boolean ENABLE_SQLITE_LOG = true;















    private static final AggregatedStats[] EMPTY_AGGREGATED_STATS = new AggregatedStats[0];















    private static final String DEVICE_GLOBAL_STATS = "__global"; // packages start with letters















    private static final int MSG_EMIT = 1;









@@ -73,12 +63,15 @@ public class NotificationUsageStats {













    public static final int FOUR_HOURS = 1000 * 60 * 60 * 4;















    private static final long EMIT_PERIOD = DEBUG ? TEN_SECONDS : FOUR_HOURS;





























    // Guarded by synchronized(this).













    @GuardedBy("this")















    private final Map<String, AggregatedStats> mStats = new HashMap<>();













    @GuardedBy("this")















    private final ArrayDeque<AggregatedStats[]> mStatsArrays = new ArrayDeque<>();













    @GuardedBy("this")















    private ArraySet<String> mStatExpiredkeys = new ArraySet<>();















    private final Context mContext;















    private final Handler mHandler;













    @GuardedBy("this")















    private long mLastEmitTime;































    public NotificationUsageStats(Context context) {









@@ -105,11 +98,7 @@ public class NotificationUsageStats {













     */















    public synchronized float getAppEnqueueRate(String packageName) {















        AggregatedStats stats = getOrCreateAggregatedStatsLocked(packageName);













        if (stats != null) {















        return stats.getEnqueueRate(SystemClock.elapsedRealtime());













        } else {













            return 0f;













        }















    }































    /**









@@ -117,11 +106,7 @@ public class NotificationUsageStats {













     */















    public synchronized boolean isAlertRateLimited(String packageName) {















        AggregatedStats stats = getOrCreateAggregatedStatsLocked(packageName);













        if (stats != null) {















        return stats.isAlertRateLimited();













        } else {













            return false;













        }















    }































    /**









@@ -135,17 +120,33 @@ public class NotificationUsageStats {













        releaseAggregatedStatsLocked(aggregatedStatsArray);















    }





























    /**













     * Called when a notification that was enqueued by an app is effectively enqueued to be













     * posted. This is after rate checking, to update the rate.













     *













     * <p>Note that if we updated the arrival estimate <em>before</em> checking it, then an app













     * enqueueing at slightly above the acceptable rate would never get their notifications













     * accepted; updating afterwards allows the rate to dip below the threshold and thus lets













     * through some of them.













     */













    public synchronized void registerEnqueuedByAppAndAccepted(String packageName) {













        final long now = SystemClock.elapsedRealtime();













        AggregatedStats[] aggregatedStatsArray = getAggregatedStatsLocked(packageName);













        for (AggregatedStats stats : aggregatedStatsArray) {













            stats.updateInterarrivalEstimate(now);













        }













        releaseAggregatedStatsLocked(aggregatedStatsArray);













    }





























    /**















     * Called when a notification has been posted.















     */















    public synchronized void registerPostedByApp(NotificationRecord notification) {













        final long now = SystemClock.elapsedRealtime();













        notification.stats.posttimeElapsedMs = now;













        notification.stats.posttimeElapsedMs = SystemClock.elapsedRealtime();































        AggregatedStats[] aggregatedStatsArray = getAggregatedStatsLocked(notification);















        for (AggregatedStats stats : aggregatedStatsArray) {















            stats.numPostedByApp++;













            stats.updateInterarrivalEstimate(now);















            stats.countApiUse(notification);















            stats.numUndecoratedRemoteViews += (notification.hasUndecoratedRemoteView() ? 1 : 0);















        }









@@ -161,7 +162,6 @@ public class NotificationUsageStats {













        AggregatedStats[] aggregatedStatsArray = getAggregatedStatsLocked(notification);















        for (AggregatedStats stats : aggregatedStatsArray) {















            stats.numUpdatedByApp++;













            stats.updateInterarrivalEstimate(SystemClock.elapsedRealtime());















            stats.countApiUse(notification);















        }















        releaseAggregatedStatsLocked(aggregatedStatsArray);










@@ -257,12 +257,12 @@ public class NotificationUsageStats {













        }















    }





























    // Locked by this.













    @GuardedBy("this")















    private AggregatedStats[] getAggregatedStatsLocked(NotificationRecord record) {















        return getAggregatedStatsLocked(record.getSbn().getPackageName());















    }





























    // Locked by this.













    @GuardedBy("this")















    private AggregatedStats[] getAggregatedStatsLocked(String packageName) {















        if (!ENABLE_AGGREGATED_IN_MEMORY_STATS) {















            return EMPTY_AGGREGATED_STATS;









@@ -277,7 +277,7 @@ public class NotificationUsageStats {













        return array;















    }





























    // Locked by this.













    @GuardedBy("this")















    private void releaseAggregatedStatsLocked(AggregatedStats[] array) {















        for(int i = 0; i < array.length; i++) {















            array[i] = null;









@@ -285,7 +285,7 @@ public class NotificationUsageStats {













        mStatsArrays.offer(array);















    }





























    // Locked by this.













    @GuardedBy("this")















    private AggregatedStats getOrCreateAggregatedStatsLocked(String key) {















        AggregatedStats result = mStats.get(key);















        if (result == null) {

































services/core/java/com/android/server/notification/RateEstimator.java



+5
−10




























Original line number
Diff line number
Diff line








@@ -22,9 +22,10 @@ package com.android.server.notification;













 *















 * {@hide}















 */













public class RateEstimator {













    private static final double RATE_ALPHA = 0.8;













class RateEstimator {













    private static final double RATE_ALPHA = 0.5;















    private static final double MINIMUM_DT = 0.0005;





























    private Long mLastEventTime;















    private double mInterarrivalTime;

























@@ -34,18 +35,12 @@ public class RateEstimator {













    }































    /** Update the estimate to account for an event that just happened. */













    public float update(long now) {













        float rate;













        if (mLastEventTime == null) {













            // No last event time, rate is zero.













            rate = 0f;













        } else {













    public void update(long now) {













        if (mLastEventTime != null) {















            // Calculate the new inter-arrival time based on last event time.















            mInterarrivalTime = getInterarrivalEstimate(now);













            rate = (float) (1.0 / mInterarrivalTime);















        }















        mLastEventTime = now;













        return rate;















    }































    /** @return the estimated rate if there were a new event right now. */

































services/tests/uiservicestests/src/com/android/server/notification/NotificationManagerServiceTest.java



+99
−0




























Original line number
Diff line number
Diff line








@@ -81,6 +81,7 @@ import static com.android.internal.config.sysui.SystemUiSystemPropertiesFlags.No













import static com.android.internal.config.sysui.SystemUiSystemPropertiesFlags.NotificationFlags.FSI_FORCE_DEMOTE;















import static com.android.internal.config.sysui.SystemUiSystemPropertiesFlags.NotificationFlags.SHOW_STICKY_HUN_FOR_DENIED_FSI;















import static com.android.internal.widget.LockPatternUtils.StrongAuthTracker.STRONG_AUTH_REQUIRED_AFTER_USER_LOCKDOWN;













import static com.android.server.notification.NotificationManagerService.DEFAULT_MAX_NOTIFICATION_ENQUEUE_RATE;















import static com.android.server.notification.NotificationRecordLogger.NotificationReportedEvent.NOTIFICATION_ADJUSTED;















import static com.android.server.notification.NotificationRecordLogger.NotificationReportedEvent.NOTIFICATION_POSTED;















import static com.android.server.notification.NotificationRecordLogger.NotificationReportedEvent.NOTIFICATION_UPDATED;










@@ -678,6 +679,7 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {



























    @After















    public void assertAllTrackersFinishedOrCancelled() {













        waitForIdle(); // Finish async work.















        // Verify that no trackers were left dangling.















        for (PostNotificationTracker tracker : mPostNotificationTrackerFactory.mCreatedTrackers) {















            assertThat(tracker.isOngoing()).isFalse();










@@ -11768,6 +11770,103 @@ public class NotificationManagerServiceTest extends UiServiceTestCase {













        assertFalse(n.isUserInitiatedJob());















    }



























    @Test













    public void enqueue_updatesEnqueueRate() throws Exception {













        Notification n = generateNotificationRecord(null).getNotification();

























        mBinderService.enqueueNotificationWithTag(PKG, PKG, "tag", 0, n, mUserId);













        // Don't waitForIdle() here. We want to verify the "intermediate" state.

























        verify(mUsageStats).registerEnqueuedByApp(eq(PKG));













        verify(mUsageStats).registerEnqueuedByAppAndAccepted(eq(PKG));













        verify(mUsageStats, never()).registerPostedByApp(any());

























        waitForIdle();













    }

























    @Test













    public void enqueue_withPost_updatesEnqueueRateAndPost() throws Exception {













        Notification n = generateNotificationRecord(null).getNotification();

























        mBinderService.enqueueNotificationWithTag(PKG, PKG, "tag", 0, n, mUserId);













        waitForIdle();

























        verify(mUsageStats).registerEnqueuedByApp(eq(PKG));













        verify(mUsageStats).registerEnqueuedByAppAndAccepted(eq(PKG));













        verify(mUsageStats).registerPostedByApp(any());













    }

























    @Test













    public void enqueueNew_whenOverEnqueueRate_accepts() throws Exception {













        Notification n = generateNotificationRecord(null).getNotification();













        when(mUsageStats.getAppEnqueueRate(eq(PKG)))













                .thenReturn(DEFAULT_MAX_NOTIFICATION_ENQUEUE_RATE + 1f);

























        mBinderService.enqueueNotificationWithTag(PKG, PKG, "tag", 0, n, mUserId);













        waitForIdle();

























        assertThat(mService.mNotificationsByKey).hasSize(1);













        verify(mUsageStats).registerEnqueuedByApp(eq(PKG));













        verify(mUsageStats).registerEnqueuedByAppAndAccepted(eq(PKG));













        verify(mUsageStats).registerPostedByApp(any());













    }

























    @Test













    public void enqueueUpdate_whenBelowMaxEnqueueRate_accepts() throws Exception {













        // Post the first version.













        Notification original = generateNotificationRecord(null).getNotification();













        original.when = 111;













        mBinderService.enqueueNotificationWithTag(PKG, PKG, "tag", 0, original, mUserId);













        waitForIdle();













        assertThat(mService.mNotificationList).hasSize(1);













        assertThat(mService.mNotificationList.get(0).getNotification().when).isEqualTo(111);

























        reset(mUsageStats);













        when(mUsageStats.getAppEnqueueRate(eq(PKG)))













                .thenReturn(DEFAULT_MAX_NOTIFICATION_ENQUEUE_RATE - 1f);

























        // Post the update.













        Notification update = generateNotificationRecord(null).getNotification();













        update.when = 222;













        mBinderService.enqueueNotificationWithTag(PKG, PKG, "tag", 0, update, mUserId);













        waitForIdle();

























        verify(mUsageStats).registerEnqueuedByApp(eq(PKG));













        verify(mUsageStats).registerEnqueuedByAppAndAccepted(eq(PKG));













        verify(mUsageStats, never()).registerPostedByApp(any());













        verify(mUsageStats).registerUpdatedByApp(any(), any());













        assertThat(mService.mNotificationList).hasSize(1);













        assertThat(mService.mNotificationList.get(0).getNotification().when).isEqualTo(222);













    }

























    @Test













    public void enqueueUpdate_whenAboveMaxEnqueueRate_rejects() throws Exception {













        // Post the first version.













        Notification original = generateNotificationRecord(null).getNotification();













        original.when = 111;













        mBinderService.enqueueNotificationWithTag(PKG, PKG, "tag", 0, original, mUserId);













        waitForIdle();













        assertThat(mService.mNotificationList).hasSize(1);













        assertThat(mService.mNotificationList.get(0).getNotification().when).isEqualTo(111);

























        reset(mUsageStats);













        when(mUsageStats.getAppEnqueueRate(eq(PKG)))













                .thenReturn(DEFAULT_MAX_NOTIFICATION_ENQUEUE_RATE + 1f);

























        // Post the update.













        Notification update = generateNotificationRecord(null).getNotification();













        update.when = 222;













        mBinderService.enqueueNotificationWithTag(PKG, PKG, "tag", 0, update, mUserId);













        waitForIdle();

























        verify(mUsageStats).registerEnqueuedByApp(eq(PKG));













        verify(mUsageStats, never()).registerEnqueuedByAppAndAccepted(any());













        verify(mUsageStats, never()).registerPostedByApp(any());













        verify(mUsageStats, never()).registerUpdatedByApp(any(), any());













        assertThat(mService.mNotificationList).hasSize(1);













        assertThat(mService.mNotificationList.get(0).getNotification().when).isEqualTo(111); // old













    }



























    private void setDpmAppOppsExemptFromDismissal(boolean isOn) {















        DeviceConfig.setProperty(















                DeviceConfig.NAMESPACE_DEVICE_POLICY_MANAGER,

























services/tests/uiservicestests/src/com/android/server/notification/RateEstimatorTest.java



+52
−42


























File changed.
Preview size limit exceeded, changes collapsed.