Fix NetworkStatsObersers thread leak in the NetworkStatsServiceTest
Test: atest ConnectivityCoverageTests:android.net.connectivity.com.android.server.net.NetworkStatsServiceTest
(with debug code that dump all threads at the end of tests)
Fix: 308544001
Change-Id: I597054633bbb008ffd0edebe34dcf6935958aa5d
diff --git a/service-t/src/com/android/server/net/NetworkStatsObservers.java b/service-t/src/com/android/server/net/NetworkStatsObservers.java
index 1cd670a..21cf351 100644
--- a/service-t/src/com/android/server/net/NetworkStatsObservers.java
+++ b/service-t/src/com/android/server/net/NetworkStatsObservers.java
@@ -142,6 +142,11 @@
@VisibleForTesting
protected Looper getHandlerLooperLocked() {
+ // TODO: Currently, callbacks are dispatched on this thread if the caller register
+ // callback without supplying a Handler. To ensure that the service handler thread
+ // is not blocked by client code, the observers must create their own thread. Once
+ // all callbacks are dispatched outside of the handler thread, the service handler
+ // thread can be used here.
HandlerThread handlerThread = new HandlerThread(TAG);
handlerThread.start();
return handlerThread.getLooper();
diff --git a/tests/unit/java/com/android/server/net/NetworkStatsObserversTest.java b/tests/unit/java/com/android/server/net/NetworkStatsObserversTest.java
index c477b2c..e62ac74 100644
--- a/tests/unit/java/com/android/server/net/NetworkStatsObserversTest.java
+++ b/tests/unit/java/com/android/server/net/NetworkStatsObserversTest.java
@@ -125,7 +125,7 @@
public void setUp() throws Exception {
MockitoAnnotations.initMocks(this);
- mObserverHandlerThread = new HandlerThread("HandlerThread");
+ mObserverHandlerThread = new HandlerThread("NetworkStatsObserversTest");
mObserverHandlerThread.start();
final Looper observerLooper = mObserverHandlerThread.getLooper();
mStatsObservers = new NetworkStatsObservers() {
diff --git a/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java b/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java
index e8d5c66..92a5b64 100644
--- a/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java
+++ b/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java
@@ -120,6 +120,7 @@
import android.os.Handler;
import android.os.HandlerThread;
import android.os.IBinder;
+import android.os.Looper;
import android.os.PowerManager;
import android.os.SimpleClock;
import android.provider.Settings;
@@ -284,6 +285,7 @@
private @Mock PersistentInt mImportLegacyFallbacksCounter;
private @Mock Resources mResources;
private Boolean mIsDebuggable;
+ private HandlerThread mObserverHandlerThread;
private class MockContext extends BroadcastInterceptingContext {
private final Context mBaseContext;
@@ -365,10 +367,23 @@
PowerManager.WakeLock wakeLock =
powerManager.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, TAG);
- mHandlerThread = new HandlerThread("HandlerThread");
+ mHandlerThread = new HandlerThread("NetworkStatsServiceTest-HandlerThread");
final NetworkStatsService.Dependencies deps = makeDependencies();
+ // Create a separate thread for observers to run on. This thread cannot be the same
+ // as the handler thread, because the observer callback is fired on this thread, and
+ // it should not be blocked by client code. Additionally, creating the observers
+ // object requires a looper, which can only be obtained after a thread has been started.
+ mObserverHandlerThread = new HandlerThread("NetworkStatsServiceTest-ObserversThread");
+ mObserverHandlerThread.start();
+ final Looper observerLooper = mObserverHandlerThread.getLooper();
+ final NetworkStatsObservers statsObservers = new NetworkStatsObservers() {
+ @Override
+ protected Looper getHandlerLooperLocked() {
+ return observerLooper;
+ }
+ };
mService = new NetworkStatsService(mServiceContext, mNetd, mAlarmManager, wakeLock,
- mClock, mSettings, mStatsFactory, new NetworkStatsObservers(), deps);
+ mClock, mSettings, mStatsFactory, statsObservers, deps);
mElapsedRealtime = 0L;
@@ -545,8 +560,14 @@
mSession.close();
mService = null;
- mHandlerThread.quitSafely();
- mHandlerThread.join();
+ if (mHandlerThread != null) {
+ mHandlerThread.quitSafely();
+ mHandlerThread.join();
+ }
+ if (mObserverHandlerThread != null) {
+ mObserverHandlerThread.quitSafely();
+ mObserverHandlerThread.join();
+ }
}
private void initWifiStats(NetworkStateSnapshot snapshot) throws Exception {