From 2c8b3e302fa2068adbd15701d4d9498fcbd551ee Mon Sep 17 00:00:00 2001
From: Chalard Jean <jchalard@google.com>
Date: Tue, 5 Nov 2019 14:40:23 +0900
Subject: [PATCH] [NS A01] Add checks for the handler thread

This is a preliminary step to removing mNetworkForRequestId, whose
role could be better managed by storing the network inside the NRI.
This serves two purposes :
1. It is a sanity check. Those functions should never be called out
of the handler thread, and if they are it's a bug.
2. It will serve to prove the followup changes are correct.

Bug: 113554781
Test: ConnectivityServiceTest
Change-Id: If29066839ad640121d33f231abdd4f37d0ad3fd5
---
 .../android/server/ConnectivityService.java    | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java
index 81eb4b355a..7c68f20fef 100644
--- a/services/core/java/com/android/server/ConnectivityService.java
+++ b/services/core/java/com/android/server/ConnectivityService.java
@@ -3050,7 +3050,8 @@ public class ConnectivityService extends IConnectivityManager.Stub
     }
 
     private void handleAsyncChannelHalfConnect(Message msg) {
-        AsyncChannel ac = (AsyncChannel) msg.obj;
+        ensureRunningOnConnectivityServiceThread();
+        final AsyncChannel ac = (AsyncChannel) msg.obj;
         if (mNetworkFactoryInfos.containsKey(msg.replyTo)) {
             if (msg.arg1 == AsyncChannel.STATUS_SUCCESSFUL) {
                 if (VDBG) log("NetworkFactory connected");
@@ -3116,6 +3117,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
     // ConnectivityService, free its interfaces and clean up.
     // Must be called on the Handler thread.
     private void disconnectAndDestroyNetwork(NetworkAgentInfo nai) {
+        ensureRunningOnConnectivityServiceThread();
         if (DBG) {
             log(nai.name() + " got DISCONNECTED, was satisfying " + nai.numNetworkRequests());
         }
@@ -3253,6 +3255,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
     }
 
     private void handleRegisterNetworkRequest(NetworkRequestInfo nri) {
+        ensureRunningOnConnectivityServiceThread();
         mNetworkRequests.put(nri.request, nri);
         mNetworkRequestInfoLogs.log("REGISTER " + nri);
         if (nri.request.isListen()) {
@@ -3286,6 +3289,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
     // - UnneededFor.LINGER: foreground NetworkRequests. If a network is unneeded for this reason,
     //   then it should be lingered.
     private boolean unneeded(NetworkAgentInfo nai, UnneededFor reason) {
+        ensureRunningOnConnectivityServiceThread();
         final int numRequests;
         switch (reason) {
             case TEARDOWN:
@@ -3344,6 +3348,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
     }
 
     private void handleTimedOutNetworkRequest(final NetworkRequestInfo nri) {
+        ensureRunningOnConnectivityServiceThread();
         if (mNetworkRequests.get(nri.request) == null) {
             return;
         }
@@ -3374,6 +3379,8 @@ public class ConnectivityService extends IConnectivityManager.Stub
     }
 
     private void handleRemoveNetworkRequest(final NetworkRequestInfo nri) {
+        ensureRunningOnConnectivityServiceThread();
+
         nri.unlinkDeathRecipient();
         mNetworkRequests.remove(nri.request);
 
@@ -5506,7 +5513,11 @@ public class ConnectivityService extends IConnectivityManager.Stub
     private final HashSet<Integer> mBlockedAppUids = new HashSet<>();
 
     // Note: if mDefaultRequest is changed, NetworkMonitor needs to be updated.
+    @NonNull
     private final NetworkRequest mDefaultRequest;
+    // The NetworkAgentInfo currently satisfying the default request, if any.
+    @Nullable
+    private volatile NetworkAgentInfo mDefaultNetworkNai = null;
 
     // Request used to optionally keep mobile data active even when higher
     // priority networks like Wi-Fi are active.
@@ -5525,17 +5536,19 @@ public class ConnectivityService extends IConnectivityManager.Stub
     private void clearNetworkForRequest(int requestId) {
         synchronized (mNetworkForRequestId) {
             mNetworkForRequestId.remove(requestId);
+            if (mDefaultRequest.requestId == requestId) mDefaultNetworkNai = null;
         }
     }
 
     private void setNetworkForRequest(int requestId, NetworkAgentInfo nai) {
         synchronized (mNetworkForRequestId) {
             mNetworkForRequestId.put(requestId, nai);
+            if (mDefaultRequest.requestId == requestId) mDefaultNetworkNai = nai;
         }
     }
 
     private NetworkAgentInfo getDefaultNetwork() {
-        return getNetworkForRequest(mDefaultRequest.requestId);
+        return mDefaultNetworkNai;
     }
 
     @Nullable
@@ -6325,6 +6338,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
     //               validated) of becoming the highest scoring network.
     private void rematchNetworkAndRequests(NetworkAgentInfo newNetwork,
             ReapUnvalidatedNetworks reapUnvalidatedNetworks, long now) {
+        ensureRunningOnConnectivityServiceThread();
         if (!newNetwork.everConnected) return;
         boolean keep = newNetwork.isVPN();
         boolean isNewDefault = false;