From 0e9e646d5b2c5988bef24a7d05e1fb6b435c3000 Mon Sep 17 00:00:00 2001
From: Lorenzo Colitti <lorenzo@google.com>
Date: Mon, 2 Mar 2020 19:24:15 +0900
Subject: [PATCH] Make NetworkFactory a single-use object.

Currently, when bluetooth tethering is disabled, the bluetooth
process crashes because unregister() calls setScoreFilter() and
immediately nulls out mProvider. This crashes as soon as the
message posted by setScoreFilter() dereferences mProvider from
the handler thread.

This sort of teardown problem is very difficult to fix given
NetworkFactory's threading model where some code runs on the
handler thread and some code runs on whatever thread calls it.
Instead of complicating the code to do this, simply make it
impossible to call register() after having called unregister().
The only factory that does so today is bluetooth tethering, and
that can be trivially modified to create and register a new
factory instead.

This CL renames the unregister() method to terminate() in order
to make it clear that the object cannot be reused, and also to
ensure that no other code in the tree is unexpectedly calling
unregister().

Bug: 148635501
Test: atest FrameworksNetTests
Change-Id: Icf4c7a957e46e4ba017c0a5984e5c42a75b36ecd
Merged-In: Icf4c7a957e46e4ba017c0a5984e5c42a75b36ecd
---
 .../android/net/NetworkFactory.java               | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/staticlibs/src_servicescommon/android/net/NetworkFactory.java b/staticlibs/src_servicescommon/android/net/NetworkFactory.java
index 47054f504a..9bcc86378a 100644
--- a/staticlibs/src_servicescommon/android/net/NetworkFactory.java
+++ b/staticlibs/src_servicescommon/android/net/NetworkFactory.java
@@ -117,10 +117,10 @@ public class NetworkFactory extends Handler {
         mCapabilityFilter = filter;
     }
 
+    /* Registers this NetworkFactory with the system. May only be called once per factory. */
     public void register() {
         if (mProvider != null) {
-            Log.e(LOG_TAG, "Ignoring attempt to register already-registered NetworkFactory");
-            return;
+            throw new IllegalStateException("A NetworkFactory must only be registered once");
         }
         if (DBG) log("Registering NetworkFactory");
 
@@ -142,16 +142,19 @@ public class NetworkFactory extends Handler {
             Context.CONNECTIVITY_SERVICE)).registerNetworkProvider(mProvider);
     }
 
-    public void unregister() {
+    /** Unregisters this NetworkFactory. After this call, the object can no longer be used. */
+    public void terminate() {
         if (mProvider == null) {
-            Log.e(LOG_TAG, "Ignoring attempt to unregister unregistered NetworkFactory");
-            return;
+            throw new IllegalStateException("This NetworkFactory was never registered");
         }
         if (DBG) log("Unregistering NetworkFactory");
 
         ((ConnectivityManager) mContext.getSystemService(Context.CONNECTIVITY_SERVICE))
             .unregisterNetworkProvider(mProvider);
-        mProvider = null;
+
+        // Remove all pending messages, since this object cannot be reused. Any message currently
+        // being processed will continue to run.
+        removeCallbacksAndMessages(null);
     }
 
     @Override