commit 9c227d9e2415f8a4a68a97f332653914dcd03716
author cosmohsieh <cosmohsieh@google.com> Mon Apr 29 14:22:51 2019 +0800
committer cosmohsieh <cosmohsieh@google.com> Mon Apr 29 15:07:30 2019 +0800
tree 7224bbef7b4fd2c41c096dc7b7a800670d2c275b
parent b09eed23095daeab6a458af9f687d397b142e882

Fix WifiTracker keeps sacnning even if onStop() is called.

In critical case, WifiTracker.pauseScanning() and updateWifiState() are called
in same time but in different threads, causes racing condition problem.
mScanner.resume() will be called after mScanner.pause() but before  mScanner
is set as null.
Adding synchronized to prevent it.

Bug: 123404912
Test: atest WifiTrackerTest
Change-Id: I3a24fb0830cd1d0c93eca03786c10a1d168de00e

packages/SettingsLib/src/com/android/settingslib/wifi/WifiTracker.java

diff --git a/packages/SettingsLib/src/com/android/settingslib/wifi/WifiTracker.java b/packages/SettingsLib/src/com/android/settingslib/wifi/WifiTracker.java
index 5f2bc4e..d8172a0 100644
--- a/packages/SettingsLib/src/com/android/settingslib/wifi/WifiTracker.java
+++ b/packages/SettingsLib/src/com/android/settingslib/wifi/WifiTracker.java
@@ -123,7 +123,8 @@
     /**
      * Synchronization lock for managing concurrency between main and worker threads.
      *
-     * <p>This lock should be held for all modifications to {@link #mInternalAccessPoints}.
+     * <p>This lock should be held for all modifications to {@link #mInternalAccessPoints} and
+     * {@link #mScanner}.
      */
     private final Object mLock = new Object();
 
@@ -168,6 +169,7 @@
     private static final String WIFI_SECURITY_OWE = "OWE";
     private static final String WIFI_SECURITY_SUITE_B_192 = "SUITE_B_192";
 
+    @GuardedBy("mLock")
     @VisibleForTesting
     Scanner mScanner;
 
@@ -276,9 +278,11 @@
      * <p>Sets {@link #mStaleScanResults} to true.
      */
     private void pauseScanning() {
-        if (mScanner != null) {
-            mScanner.pause();
-            mScanner = null;
+        synchronized (mLock) {
+            if (mScanner != null) {
+                mScanner.pause();
+                mScanner = null;
+            }
         }
         mStaleScanResults = true;
     }
@@ -289,12 +293,14 @@
      * <p>The score cache should be registered before this method is invoked.
      */
     public void resumeScanning() {
-        if (mScanner == null) {
-            mScanner = new Scanner();
-        }
+        synchronized (mLock) {
+            if (mScanner == null) {
+                mScanner = new Scanner();
+            }
 
-        if (isWifiEnabled()) {
-            mScanner.resume();
+            if (isWifiEnabled()) {
+                mScanner.resume();
+            }
         }
     }
 
@@ -743,7 +749,6 @@
     }
 
     private void updateNetworkInfo(NetworkInfo networkInfo) {
-
         /* Sticky broadcasts can call this when wifi is disabled */
         if (!isWifiEnabled()) {
             clearAccessPointsAndConditionallyUpdate();
@@ -880,18 +885,25 @@
      * true.
      */
     private void updateWifiState(int state) {
+        if (isVerboseLoggingEnabled()) {
+            Log.d(TAG, "updateWifiState: " + state);
+        }
         if (state == WifiManager.WIFI_STATE_ENABLED) {
-            if (mScanner != null) {
-                // We only need to resume if mScanner isn't null because
-                // that means we want to be scanning.
-                mScanner.resume();
+            synchronized (mLock) {
+                if (mScanner != null) {
+                    // We only need to resume if mScanner isn't null because
+                    // that means we want to be scanning.
+                    mScanner.resume();
+                }
             }
         } else {
             clearAccessPointsAndConditionallyUpdate();
             mLastInfo = null;
             mLastNetworkInfo = null;
-            if (mScanner != null) {
-                mScanner.pause();
+            synchronized (mLock) {
+                if (mScanner != null) {
+                    mScanner.pause();
+                }
             }
             mStaleScanResults = true;
         }
@@ -919,12 +931,18 @@
         private int mRetry = 0;
 
         void resume() {
+            if (isVerboseLoggingEnabled()) {
+                Log.d(TAG, "Scanner resume");
+            }
             if (!hasMessages(MSG_SCAN)) {
                 sendEmptyMessage(MSG_SCAN);
             }
         }
 
         void pause() {
+            if (isVerboseLoggingEnabled()) {
+                Log.d(TAG, "Scanner pause");
+            }
             mRetry = 0;
             removeMessages(MSG_SCAN);
         }