Merge "Limit SelfRecovery wifi restarts to some amount" into oc-dr1-dev

3 files changed, 115 insertions, 6 deletions

diff --git a/service/java/com/android/server/wifi/SelfRecovery.java b/service/java/com/android/server/wifi/SelfRecovery.java
index b35b7ccce..21a3e0ac7 100644
--- a/service/java/com/android/server/wifi/SelfRecovery.java
+++ b/service/java/com/android/server/wifi/SelfRecovery.java
@@ -18,6 +18,9 @@ package com.android.server.wifi;
 
 import android.util.Log;
 
+import java.util.Iterator;
+import java.util.LinkedList;
+
 /**
  * This class is used to recover the wifi stack from a fatal failure. The recovery mechanism
  * involves triggering a stack restart (essentially simulating an airplane mode toggle) using
@@ -36,7 +39,8 @@ public class SelfRecovery {
     public static final int REASON_LAST_RESORT_WATCHDOG = 0;
     public static final int REASON_HAL_CRASH = 1;
     public static final int REASON_WIFICOND_CRASH = 2;
-
+    public static final long MAX_RESTARTS_IN_TIME_WINDOW = 2; // 2 restarts per hour
+    public static final long MAX_RESTARTS_TIME_WINDOW_MILLIS = 60 * 60 * 1000; // 1 hour
     private static final String[] REASON_STRINGS = {
             "Last Resort Watchdog", // REASON_LAST_RESORT_WATCHDOG
             "Hal Crash",            // REASON_HAL_CRASH
@@ -44,9 +48,13 @@ public class SelfRecovery {
     };
 
     private final WifiController mWifiController;
-
-    SelfRecovery(WifiController wifiController) {
+    private final Clock mClock;
+    // Time since boot (in millis) that restart occurred
+    private final LinkedList<Long> mPastRestartTimes;
+    public SelfRecovery(WifiController wifiController, Clock clock) {
         mWifiController = wifiController;
+        mClock = clock;
+        mPastRestartTimes = new LinkedList<Long>();
     }
 
     /**
@@ -59,11 +67,38 @@ public class SelfRecovery {
      * @param reason One of the above |REASON_*| codes.
      */
     public void trigger(int reason) {
-        if (reason < REASON_LAST_RESORT_WATCHDOG || reason > REASON_WIFICOND_CRASH) {
+        if (!(reason == REASON_LAST_RESORT_WATCHDOG || reason == REASON_HAL_CRASH
+                || reason == REASON_WIFICOND_CRASH)) {
             Log.e(TAG, "Invalid trigger reason. Ignoring...");
             return;
         }
         Log.wtf(TAG, "Triggering recovery for reason: " + REASON_STRINGS[reason]);
+        if (reason == REASON_WIFICOND_CRASH || reason == REASON_HAL_CRASH) {
+            trimPastRestartTimes();
+            // Ensure there haven't been too many restarts within MAX_RESTARTS_TIME_WINDOW
+            if (mPastRestartTimes.size() >= MAX_RESTARTS_IN_TIME_WINDOW) {
+                Log.e(TAG, "Already restarted wifi (" + MAX_RESTARTS_IN_TIME_WINDOW + ") times in"
+                        + " last (" + MAX_RESTARTS_TIME_WINDOW_MILLIS + "ms ). Ignoring...");
+                return;
+            }
+            mPastRestartTimes.add(mClock.getElapsedSinceBootMillis());
+        }
         mWifiController.sendMessage(WifiController.CMD_RESTART_WIFI);
     }
+
+    /**
+     * Process the mPastRestartTimes list, removing elements outside the max restarts time window
+     */
+    private void trimPastRestartTimes() {
+        Iterator<Long> iter = mPastRestartTimes.iterator();
+        long now = mClock.getElapsedSinceBootMillis();
+        while (iter.hasNext()) {
+            Long restartTimeMillis = iter.next();
+            if (now - restartTimeMillis > MAX_RESTARTS_TIME_WINDOW_MILLIS) {
+                iter.remove();
+            } else {
+                break;
+            }
+        }
+    }
 }
diff --git a/service/java/com/android/server/wifi/WifiInjector.java b/service/java/com/android/server/wifi/WifiInjector.java
index fd7222e0c..c4be62570 100644
--- a/service/java/com/android/server/wifi/WifiInjector.java
+++ b/service/java/com/android/server/wifi/WifiInjector.java
@@ -230,7 +230,7 @@ public class WifiInjector {
         mLockManager = new WifiLockManager(mContext, BatteryStatsService.getService());
         mWifiController = new WifiController(mContext, mWifiStateMachine, mSettingsStore,
                 mLockManager, mWifiServiceHandlerThread.getLooper(), mFrameworkFacade);
-        mSelfRecovery = new SelfRecovery(mWifiController);
+        mSelfRecovery = new SelfRecovery(mWifiController, mClock);
         mWifiLastResortWatchdog = new WifiLastResortWatchdog(mSelfRecovery, mWifiMetrics);
         mWifiMulticastLockManager = new WifiMulticastLockManager(mWifiStateMachine,
                 BatteryStatsService.getService());
diff --git a/tests/wifitests/src/com/android/server/wifi/SelfRecoveryTest.java b/tests/wifitests/src/com/android/server/wifi/SelfRecoveryTest.java
index 0e55b72a9..c8b93e95d 100644
--- a/tests/wifitests/src/com/android/server/wifi/SelfRecoveryTest.java
+++ b/tests/wifitests/src/com/android/server/wifi/SelfRecoveryTest.java
@@ -32,11 +32,12 @@ import org.mockito.Mock;
 public class SelfRecoveryTest {
     SelfRecovery mSelfRecovery;
     @Mock WifiController mWifiController;
+    @Mock Clock mClock;
 
     @Before
     public void setUp() throws Exception {
         initMocks(this);
-        mSelfRecovery = new SelfRecovery(mWifiController);
+        mSelfRecovery = new SelfRecovery(mWifiController, mClock);
     }
 
     /**
@@ -49,10 +50,14 @@ public class SelfRecoveryTest {
         verify(mWifiController).sendMessage(eq(WifiController.CMD_RESTART_WIFI));
         reset(mWifiController);
 
+        when(mClock.getElapsedSinceBootMillis())
+                .thenReturn(SelfRecovery.MAX_RESTARTS_TIME_WINDOW_MILLIS + 1);
         mSelfRecovery.trigger(SelfRecovery.REASON_HAL_CRASH);
         verify(mWifiController).sendMessage(eq(WifiController.CMD_RESTART_WIFI));
         reset(mWifiController);
 
+        when(mClock.getElapsedSinceBootMillis())
+                .thenReturn(2 * (SelfRecovery.MAX_RESTARTS_TIME_WINDOW_MILLIS + 1));
         mSelfRecovery.trigger(SelfRecovery.REASON_WIFICOND_CRASH);
         verify(mWifiController).sendMessage(eq(WifiController.CMD_RESTART_WIFI));
         reset(mWifiController);
@@ -71,4 +76,73 @@ public class SelfRecoveryTest {
         mSelfRecovery.trigger(8);
         verify(mWifiController, never()).sendMessage(anyInt());
     }
+
+    /**
+     * Verifies that invocations of {@link SelfRecovery#trigger(int)} for REASON_HAL_CRASH &
+     * REASON_WIFICOND_CRASH are limited to {@link SelfRecovery#MAX_RESTARTS_IN_TIME_WINDOW} in a
+     * {@link SelfRecovery#MAX_RESTARTS_TIME_WINDOW_MILLIS} millisecond time window.
+     */
+    @Test
+    public void testTimeWindowLimiting_typicalUse() {
+        when(mClock.getElapsedSinceBootMillis()).thenReturn(0L);
+        // Fill up the SelfRecovery's restart time window buffer, ensure all the restart triggers
+        // aren't ignored
+        for (int i = 0; i < SelfRecovery.MAX_RESTARTS_IN_TIME_WINDOW / 2; i++) {
+            mSelfRecovery.trigger(SelfRecovery.REASON_HAL_CRASH);
+            verify(mWifiController).sendMessage(eq(WifiController.CMD_RESTART_WIFI));
+            reset(mWifiController);
+
+            mSelfRecovery.trigger(SelfRecovery.REASON_WIFICOND_CRASH);
+            verify(mWifiController).sendMessage(eq(WifiController.CMD_RESTART_WIFI));
+            reset(mWifiController);
+        }
+        if ((SelfRecovery.MAX_RESTARTS_IN_TIME_WINDOW % 2) == 1) {
+            mSelfRecovery.trigger(SelfRecovery.REASON_WIFICOND_CRASH);
+            verify(mWifiController).sendMessage(eq(WifiController.CMD_RESTART_WIFI));
+            reset(mWifiController);
+        }
+
+        // Verify that further attempts to trigger restarts for are ignored
+        mSelfRecovery.trigger(SelfRecovery.REASON_HAL_CRASH);
+        verify(mWifiController, never()).sendMessage(eq(WifiController.CMD_RESTART_WIFI));
+        reset(mWifiController);
+
+        mSelfRecovery.trigger(SelfRecovery.REASON_WIFICOND_CRASH);
+        verify(mWifiController, never()).sendMessage(eq(WifiController.CMD_RESTART_WIFI));
+        reset(mWifiController);
+
+        // Verify L.R.Watchdog can still restart things (It has its own complex limiter)
+        mSelfRecovery.trigger(SelfRecovery.REASON_LAST_RESORT_WATCHDOG);
+        verify(mWifiController).sendMessage(eq(WifiController.CMD_RESTART_WIFI));
+        reset(mWifiController);
+
+        // now TRAVEL FORWARDS IN TIME and ensure that more restarts can occur
+        when(mClock.getElapsedSinceBootMillis())
+                .thenReturn(SelfRecovery.MAX_RESTARTS_TIME_WINDOW_MILLIS + 1);
+        mSelfRecovery.trigger(SelfRecovery.REASON_LAST_RESORT_WATCHDOG);
+        verify(mWifiController).sendMessage(eq(WifiController.CMD_RESTART_WIFI));
+        reset(mWifiController);
+
+        when(mClock.getElapsedSinceBootMillis())
+                .thenReturn(SelfRecovery.MAX_RESTARTS_TIME_WINDOW_MILLIS + 1);
+        mSelfRecovery.trigger(SelfRecovery.REASON_HAL_CRASH);
+        verify(mWifiController).sendMessage(eq(WifiController.CMD_RESTART_WIFI));
+        reset(mWifiController);
+    }
+
+    /**
+     * Verifies that invocations of {@link SelfRecovery#trigger(int)} for
+     * REASON_LAST_RESORT_WATCHDOG are NOT limited to
+     * {@link SelfRecovery#MAX_RESTARTS_IN_TIME_WINDOW} in a
+     * {@link SelfRecovery#MAX_RESTARTS_TIME_WINDOW_MILLIS} millisecond time window.
+     */
+    @Test
+    public void testTimeWindowLimiting_lastResortWatchdog_noEffect() {
+        for (int i = 0; i < SelfRecovery.MAX_RESTARTS_IN_TIME_WINDOW * 2; i++) {
+            // Verify L.R.Watchdog can still restart things (It has it's own complex limiter)
+            mSelfRecovery.trigger(SelfRecovery.REASON_LAST_RESORT_WATCHDOG);
+            verify(mWifiController).sendMessage(eq(WifiController.CMD_RESTART_WIFI));
+            reset(mWifiController);
+        }
+    }
 }