From 470ae84f66ce28031fb8adc14068db69429d385d Mon Sep 17 00:00:00 2001
From: Evan Laird <evanlaird@google.com>
Date: Tue, 9 Apr 2019 16:24:27 -0400
Subject: [PATCH] Guard against creating too many MobileSignalControllers

The PhoneStateListener that got added to NetworkControllerImpl used the
default (main) looper for its callbacks, which caused race conditions
when updating subscriptions. This resulted in zombie
MobileSignalControllers that were untracked and never stopped listening
for their updates.

To fix the problem we put the phone state listener on the same
background thread as the receiver handler, and also lock around updating
the subscriptions so that we can all have peace of mind.

This should fix a host of issues where the mobile signal is incorrectly
showing state such as disconnected or not showing the data type
indicator.

Fixes: 129717207
Test: visual; remove and insert sim and verify that the proper SIM state
is shown

Change-Id: Iace6a04c0629e24d2ef9c980a8de336a225d0f36
---
 .../policy/NetworkControllerImpl.java         | 26 ++++++++++++-------
 .../policy/NetworkControllerSignalTest.java   |  2 +-
 2 files changed, 17 insertions(+), 11 deletions(-)

diff --git a/packages/SystemUI/src/com/android/systemui/statusbar/policy/NetworkControllerImpl.java b/packages/SystemUI/src/com/android/systemui/statusbar/policy/NetworkControllerImpl.java
index d01430a97783e..37ab68f1daa29 100644
--- a/packages/SystemUI/src/com/android/systemui/statusbar/policy/NetworkControllerImpl.java
+++ b/packages/SystemUI/src/com/android/systemui/statusbar/policy/NetworkControllerImpl.java
@@ -55,6 +55,7 @@ import android.util.Log;
 import android.util.MathUtils;
 import android.util.SparseArray;
 
+import com.android.internal.annotations.GuardedBy;
 import com.android.internal.annotations.VisibleForTesting;
 import com.android.internal.telephony.PhoneConstants;
 import com.android.internal.telephony.TelephonyIntents;
@@ -108,16 +109,10 @@ public class NetworkControllerImpl extends BroadcastReceiver
     private final SubscriptionDefaults mSubDefaults;
     private final DataSaverController mDataSaverController;
     private final CurrentUserTracker mUserTracker;
+    private final Object mLock = new Object();
     private Config mConfig;
 
-    private PhoneStateListener mPhoneStateListener = new PhoneStateListener() {
-        @Override
-        public void onActiveDataSubscriptionIdChanged(int subId) {
-            mActiveMobileDataSubscription = subId;
-            doUpdateMobileControllers();
-        }
-    };
-
+    private PhoneStateListener mPhoneStateListener;
     private int mActiveMobileDataSubscription = SubscriptionManager.INVALID_SUBSCRIPTION_ID;
 
     // Subcontrollers.
@@ -279,6 +274,14 @@ public class NetworkControllerImpl extends BroadcastReceiver
         // TODO: Move off of the deprecated CONNECTIVITY_ACTION broadcast and rely on callbacks
         // exclusively for status bar icons.
         mConnectivityManager.registerDefaultNetworkCallback(callback, mReceiverHandler);
+        // Register the listener on our bg looper
+        mPhoneStateListener = new PhoneStateListener(bgLooper) {
+            @Override
+            public void onActiveDataSubscriptionIdChanged(int subId) {
+                mActiveMobileDataSubscription = subId;
+                doUpdateMobileControllers();
+            }
+        };
     }
 
     public DataSaverController getDataSaverController() {
@@ -600,7 +603,9 @@ public class NetworkControllerImpl extends BroadcastReceiver
             updateNoSims();
             return;
         }
-        setCurrentSubscriptions(subscriptions);
+        synchronized (mLock) {
+            setCurrentSubscriptionsLocked(subscriptions);
+        }
         updateNoSims();
         recalculateEmergency();
     }
@@ -628,8 +633,9 @@ public class NetworkControllerImpl extends BroadcastReceiver
         return false;
     }
 
+    @GuardedBy("mLock")
     @VisibleForTesting
-    void setCurrentSubscriptions(List<SubscriptionInfo> subscriptions) {
+    public void setCurrentSubscriptionsLocked(List<SubscriptionInfo> subscriptions) {
         Collections.sort(subscriptions, new Comparator<SubscriptionInfo>() {
             @Override
             public int compare(SubscriptionInfo lhs, SubscriptionInfo rhs) {
diff --git a/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/NetworkControllerSignalTest.java b/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/NetworkControllerSignalTest.java
index ac6544e129b4d..0b53c486356fa 100644
--- a/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/NetworkControllerSignalTest.java
+++ b/packages/SystemUI/tests/src/com/android/systemui/statusbar/policy/NetworkControllerSignalTest.java
@@ -300,7 +300,7 @@ public class NetworkControllerSignalTest extends NetworkControllerBaseTest {
         // We can only test whether unregister gets called if it thinks its in a listening
         // state.
         mNetworkController.mListening = true;
-        mNetworkController.setCurrentSubscriptions(subscriptions);
+        mNetworkController.setCurrentSubscriptionsLocked(subscriptions);
 
         for (int i = 0; i < testSubscriptions.length; i++) {
             if (i == indexToSkipController) {