commit 14a2bbf857fcadc9c809914a6e0d018bd6a231b0
author Lorenzo Colitti <lorenzo@google.com> Mon Mar 02 19:24:15 2020 +0900
committer Lorenzo Colitti <lorenzo@google.com> Fri Mar 13 15:30:55 2020 +0900
tree c6aa955dbe5bdd942d9526894b15f549203e26d3
parent 381392b397ba2b8c569e954eac612b72e81afac9

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

staticlibs/src_servicescommon/android/net/NetworkFactory.java

diff --git a/staticlibs/src_servicescommon/android/net/NetworkFactory.java b/staticlibs/src_servicescommon/android/net/NetworkFactory.java
index 47054f5..9bcc863 100644
--- a/staticlibs/src_servicescommon/android/net/NetworkFactory.java
+++ b/staticlibs/src_servicescommon/android/net/NetworkFactory.java
@@ -117,10 +117,10 @@
         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 @@
             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