Merge "Unregister the tethering internal callback in finalize" am: f8d1f3d1cd am: c472eed19a

Original change: https://android-review.googlesource.com/c/platform/packages/modules/Connectivity/+/1838543

Change-Id: I84a8f72bec2d7559e938d16f30716e957018d3c8

Tethering/common/TetheringLib/src/android/net/TetheringManager.java

@@ -290,6 +290,23 @@ public class TetheringManager {
         getConnector(c -> c.registerTetheringEventCallback(mCallback, pkgName));
     }
 
+    /** @hide */
+    @Override
+    protected void finalize() throws Throwable {
+        final String pkgName = mContext.getOpPackageName();
+        Log.i(TAG, "unregisterTetheringEventCallback:" + pkgName);
+        // 1. It's generally not recommended to perform long operations in finalize, but while
+        // unregisterTetheringEventCallback does an IPC, it's a oneway IPC so should not block.
+        // 2. If the connector is not yet connected, TetheringManager is impossible to finalize
+        // because the connector polling thread strong reference the TetheringManager object. So
+        // it's guaranteed that registerTetheringEventCallback was already called before calling
+        // unregisterTetheringEventCallback in finalize.
+        if (mConnector == null) Log.wtf(TAG, "null connector in finalize!");
+        getConnector(c -> c.unregisterTetheringEventCallback(mCallback, pkgName));
+
+        super.finalize();
+    }
+
     private void startPollingForConnector() {
         new Thread(() -> {
             while (true) {

Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringServiceTest.java

@@ -577,8 +577,7 @@ public final class TetheringServiceTest {
             assertNull("TetheringManager weak reference still not null after " + attempts
                     + " attempts", weakTm.get());
 
-            // BUG: internal callback do not be unregistered after TetheringManager is GCed.
+            assertEquals("Internal callback is not unregistered", 0, callbacks.size());
-            assertEquals(1, callbacks.size());
         });
     }
 }