Fix leak of NetworkAgentWrapper HandlerThread

ConnectivityServiceTest creates hundreds of NetworkAgentWrapper, without
stopping their HandlerThread. This leaks a lot of threads while tests
are running.

Ensure the threads are stopped in ConnectivityServiceTest, and
integration tests that also use NetworkAgentWrapper.

Test: atest, watch adb shell ps -T [test package pid]
Bug: 310659032
Change-Id: I34f367b2efe3d05e1db5eab4c9fd93ee79c1b683
diff --git a/tests/integration/src/com/android/server/net/integrationtests/ConnectivityServiceIntegrationTest.kt b/tests/integration/src/com/android/server/net/integrationtests/ConnectivityServiceIntegrationTest.kt
index 496d163..76d30e6 100644
--- a/tests/integration/src/com/android/server/net/integrationtests/ConnectivityServiceIntegrationTest.kt
+++ b/tests/integration/src/com/android/server/net/integrationtests/ConnectivityServiceIntegrationTest.kt
@@ -56,9 +56,11 @@
 import com.android.server.connectivity.MockableSystemProperties
 import com.android.server.connectivity.MultinetworkPolicyTracker
 import com.android.server.connectivity.ProxyTracker
+import com.android.testutils.DevSdkIgnoreRunner
 import com.android.testutils.DeviceInfoUtils
 import com.android.testutils.RecorderCallback.CallbackEntry.LinkPropertiesChanged
 import com.android.testutils.TestableNetworkCallback
+import com.android.testutils.tryTest
 import kotlin.test.assertEquals
 import kotlin.test.assertNotNull
 import kotlin.test.assertTrue
@@ -254,13 +256,18 @@
         na.addCapability(NET_CAPABILITY_INTERNET)
         na.connect()
 
-        testCallback.expectAvailableThenValidatedCallbacks(na.network, TEST_TIMEOUT_MS)
-        val requestedSize = nsInstrumentation.getRequestUrls().size
-        if (requestedSize == 2 || (requestedSize == 1 &&
-                nsInstrumentation.getRequestUrls()[0] == httpsProbeUrl)) {
-            return
+        tryTest {
+            testCallback.expectAvailableThenValidatedCallbacks(na.network, TEST_TIMEOUT_MS)
+            val requestedSize = nsInstrumentation.getRequestUrls().size
+            if (requestedSize == 2 || (requestedSize == 1 &&
+                        nsInstrumentation.getRequestUrls()[0] == httpsProbeUrl)
+            ) {
+                return@tryTest
+            }
+            fail("Unexpected request urls: ${nsInstrumentation.getRequestUrls()}")
+        } cleanup {
+            na.destroy()
         }
-        fail("Unexpected request urls: ${nsInstrumentation.getRequestUrls()}")
     }
 
     @Test
@@ -292,24 +299,32 @@
         val lp = LinkProperties()
         lp.captivePortalApiUrl = Uri.parse(apiUrl)
         val na = NetworkAgentWrapper(TRANSPORT_CELLULAR, lp, null /* ncTemplate */, context)
-        networkStackClient.verifyNetworkMonitorCreated(na.network, TEST_TIMEOUT_MS)
 
-        na.addCapability(NET_CAPABILITY_INTERNET)
-        na.connect()
+        tryTest {
+            networkStackClient.verifyNetworkMonitorCreated(na.network, TEST_TIMEOUT_MS)
 
-        testCb.expectAvailableCallbacks(na.network, validated = false, tmt = TEST_TIMEOUT_MS)
+            na.addCapability(NET_CAPABILITY_INTERNET)
+            na.connect()
 
-        val capportData = testCb.expect<LinkPropertiesChanged>(na, TEST_TIMEOUT_MS) {
-            it.lp.captivePortalData != null
-        }.lp.captivePortalData
-        assertNotNull(capportData)
-        assertTrue(capportData.isCaptive)
-        assertEquals(Uri.parse("https://login.capport.android.com"), capportData.userPortalUrl)
-        assertEquals(Uri.parse("https://venueinfo.capport.android.com"), capportData.venueInfoUrl)
+            testCb.expectAvailableCallbacks(na.network, validated = false, tmt = TEST_TIMEOUT_MS)
 
-        testCb.expectCaps(na, TEST_TIMEOUT_MS) {
-            it.hasCapability(NET_CAPABILITY_CAPTIVE_PORTAL) &&
-                    !it.hasCapability(NET_CAPABILITY_VALIDATED)
+            val capportData = testCb.expect<LinkPropertiesChanged>(na, TEST_TIMEOUT_MS) {
+                it.lp.captivePortalData != null
+            }.lp.captivePortalData
+            assertNotNull(capportData)
+            assertTrue(capportData.isCaptive)
+            assertEquals(Uri.parse("https://login.capport.android.com"), capportData.userPortalUrl)
+            assertEquals(
+                Uri.parse("https://venueinfo.capport.android.com"),
+                capportData.venueInfoUrl
+            )
+
+            testCb.expectCaps(na, TEST_TIMEOUT_MS) {
+                it.hasCapability(NET_CAPABILITY_CAPTIVE_PORTAL) &&
+                        !it.hasCapability(NET_CAPABILITY_VALIDATED)
+            }
+        } cleanup {
+            na.destroy()
         }
     }
 
diff --git a/tests/integration/util/com/android/server/NetworkAgentWrapper.java b/tests/integration/util/com/android/server/NetworkAgentWrapper.java
index ec09f9e..960c6ca 100644
--- a/tests/integration/util/com/android/server/NetworkAgentWrapper.java
+++ b/tests/integration/util/com/android/server/NetworkAgentWrapper.java
@@ -36,6 +36,7 @@
 import static org.junit.Assert.fail;
 
 import android.annotation.NonNull;
+import android.annotation.SuppressLint;
 import android.content.Context;
 import android.net.ConnectivityManager;
 import android.net.LinkProperties;
@@ -51,6 +52,7 @@
 import android.os.ConditionVariable;
 import android.os.HandlerThread;
 import android.os.Message;
+import android.util.CloseGuard;
 import android.util.Log;
 import android.util.Range;
 
@@ -65,11 +67,14 @@
 import java.util.function.Consumer;
 
 public class NetworkAgentWrapper implements TestableNetworkCallback.HasNetwork {
+    private static final long DESTROY_TIMEOUT_MS = 10_000L;
+
     // Note : Please do not add any new instrumentation here. If you need new instrumentation,
     // please add it in CSAgentWrapper and use subclasses of CSTest instead of adding more
     // tools in ConnectivityServiceTest.
     private final NetworkCapabilities mNetworkCapabilities;
     private final HandlerThread mHandlerThread;
+    private final CloseGuard mCloseGuard;
     private final Context mContext;
     private final String mLogTag;
     private final NetworkAgentConfig mNetworkAgentConfig;
@@ -157,6 +162,8 @@
         mLogTag = "Mock-" + typeName;
         mHandlerThread = new HandlerThread(mLogTag);
         mHandlerThread.start();
+        mCloseGuard = new CloseGuard();
+        mCloseGuard.open("destroy");
 
         // extraInfo is set to "" by default in NetworkAgentConfig.
         final String extraInfo = (transport == TRANSPORT_CELLULAR) ? "internet.apn" : "";
@@ -359,6 +366,35 @@
         mNetworkAgent.unregister();
     }
 
+    /**
+     * Destroy the network agent and stop its looper.
+     *
+     * <p>This must always be called.
+     */
+    public void destroy() {
+        mHandlerThread.quitSafely();
+        try {
+            mHandlerThread.join(DESTROY_TIMEOUT_MS);
+        } catch (InterruptedException e) {
+            Log.e(mLogTag, "Interrupted when waiting for handler thread on destroy", e);
+        }
+        mCloseGuard.close();
+    }
+
+    @SuppressLint("Finalize") // Follows the recommended pattern for CloseGuard
+    @Override
+    protected void finalize() throws Throwable {
+        try {
+            // Note that mCloseGuard could be null if the constructor threw.
+            if (mCloseGuard != null) {
+                mCloseGuard.warnIfOpen();
+            }
+            destroy();
+        } finally {
+            super.finalize();
+        }
+    }
+
     @Override
     public Network getNetwork() {
         return mNetworkAgent.getNetwork();
diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java
index 8f5fd7c..c681356 100755
--- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java
+++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java
@@ -589,6 +589,7 @@
     private TestNetworkAgentWrapper mWiFiAgent;
     private TestNetworkAgentWrapper mCellAgent;
     private TestNetworkAgentWrapper mEthernetAgent;
+    private final List<TestNetworkAgentWrapper> mCreatedAgents = new ArrayList<>();
     private MockVpn mMockVpn;
     private Context mContext;
     private NetworkPolicyCallback mPolicyCallback;
@@ -1092,6 +1093,7 @@
                 NetworkCapabilities ncTemplate, NetworkProvider provider,
                 NetworkAgentWrapper.Callbacks callbacks) throws Exception {
             super(transport, linkProperties, ncTemplate, provider, callbacks, mServiceContext);
+            mCreatedAgents.add(this);
 
             // Waits for the NetworkAgent to be registered, which includes the creation of the
             // NetworkMonitor.
@@ -2404,6 +2406,11 @@
         FakeSettingsProvider.clearSettingsProvider();
         ConnectivityResources.setResourcesContextForTest(null);
 
+        for (TestNetworkAgentWrapper agent : mCreatedAgents) {
+            agent.destroy();
+        }
+        mCreatedAgents.clear();
+
         mCsHandlerThread.quitSafely();
         mCsHandlerThread.join();
         mAlarmManagerThread.quitSafely();