From c714bf6d8b14afbcedefae1ecb28e09efad5b852 Mon Sep 17 00:00:00 2001
From: Paul Hu <paulhu@google.com>
Date: Tue, 1 Oct 2024 14:58:46 +0000
Subject: [PATCH] Add MonitorThreadLeak to TetheringMetricsTest

TetheringMetrics creates a thread to process all metrics work on
a handler. This thread should remain active alongside the
Tethering service but should be closed in test class. Therefore,
add MonitorThreadLeak to ensure the thread is closed correctly.

Test: atest TetheringMetricsTest
Change-Id: Ic57beb3324945ddbb45fec9697056889044e680c
---
 .../tethering/metrics/TetheringMetrics.java        | 14 ++++++++------
 .../tethering/metrics/TetheringMetricsTest.java    |  7 ++++---
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/Tethering/src/com/android/networkstack/tethering/metrics/TetheringMetrics.java b/Tethering/src/com/android/networkstack/tethering/metrics/TetheringMetrics.java
index 32c1410e09..6de40629b9 100644
--- a/Tethering/src/com/android/networkstack/tethering/metrics/TetheringMetrics.java
+++ b/Tethering/src/com/android/networkstack/tethering/metrics/TetheringMetrics.java
@@ -62,7 +62,6 @@ import android.net.NetworkCapabilities;
 import android.net.NetworkTemplate;
 import android.os.Handler;
 import android.os.HandlerThread;
-import android.os.Looper;
 import android.stats.connectivity.DownstreamType;
 import android.stats.connectivity.ErrorCode;
 import android.stats.connectivity.UpstreamType;
@@ -161,10 +160,15 @@ public class TetheringMetrics {
 
         /**
          * @see Handler
+         *
+         * Note: This should only be called once, within the constructor, as it creates a new
+         * thread. Calling it multiple times could lead to a thread leak.
          */
         @NonNull
-        public Handler createHandler(Looper looper) {
-            return new Handler(looper);
+        public Handler createHandler() {
+            final HandlerThread thread = new HandlerThread(TAG);
+            thread.start();
+            return new Handler(thread.getLooper());
         }
     }
 
@@ -181,9 +185,7 @@ public class TetheringMetrics {
         mContext = context;
         mDependencies = dependencies;
         mNetworkStatsManager = mContext.getSystemService(NetworkStatsManager.class);
-        final HandlerThread thread = new HandlerThread(TAG);
-        thread.start();
-        mHandler = dependencies.createHandler(thread.getLooper());
+        mHandler = dependencies.createHandler();
     }
 
     @VisibleForTesting
diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/metrics/TetheringMetricsTest.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/metrics/TetheringMetricsTest.java
index f736dbf66e..6b646ec542 100644
--- a/Tethering/tests/unit/src/com/android/networkstack/tethering/metrics/TetheringMetricsTest.java
+++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/metrics/TetheringMetricsTest.java
@@ -87,13 +87,13 @@ import android.stats.connectivity.UserType;
 import android.util.ArrayMap;
 
 import androidx.test.filters.SmallTest;
-import androidx.test.runner.AndroidJUnit4;
 
 import com.android.networkstack.tethering.UpstreamNetworkState;
 import com.android.networkstack.tethering.metrics.TetheringMetrics.DataUsage;
 import com.android.networkstack.tethering.metrics.TetheringMetrics.Dependencies;
 import com.android.testutils.DevSdkIgnoreRule;
 import com.android.testutils.DevSdkIgnoreRule.IgnoreUpTo;
+import com.android.testutils.DevSdkIgnoreRunner;
 import com.android.testutils.HandlerUtils;
 
 import org.junit.After;
@@ -104,7 +104,8 @@ import org.junit.runner.RunWith;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
 
-@RunWith(AndroidJUnit4.class)
+@DevSdkIgnoreRunner.MonitorThreadLeak
+@RunWith(DevSdkIgnoreRunner.class)
 @SmallTest
 public final class TetheringMetricsTest {
     @Rule public final DevSdkIgnoreRule mIgnoreRule = new DevSdkIgnoreRule();
@@ -158,7 +159,7 @@ public final class TetheringMetricsTest {
         mThread = new HandlerThread("TetheringMetricsTest");
         mThread.start();
         mHandler = new Handler(mThread.getLooper());
-        doReturn(mHandler).when(mDeps).createHandler(any());
+        doReturn(mHandler).when(mDeps).createHandler();
         // Set up the usage for upstream types.
         mMockUpstreamUsageBaseline.put(UT_CELLULAR, new DataUsage(100L, 200L));
         mMockUpstreamUsageBaseline.put(UT_WIFI, new DataUsage(400L, 800L));