Use Service handler thread for NetworkStatsObservers
This change includes:
1. Use service handler looper when initializing the observers
to simplify the structure.
2. Make the handler variable final in the observers class.
3. Remove misleading comments, where the callback events
are actually dispatched on the oneway binder thread,
which does not block handler thread.
4. Update tests accordingly.
Test: atest ConnectivityCoverageTests:android.net.connectivity.com.android.server.net.NetworkStatsServiceTest
Test: atest FrameworksNetIntegrationTests:com.android.server.net.integrationtests.NetworkStatsIntegrationTest
Test: atest CtsNetTestCases:NetworkStatsManagerTest
Bug: N/A
Change-Id: I5be92853b5bbc795a36bf774956bdac33d79099b
diff --git a/service-t/src/com/android/server/net/NetworkStatsObservers.java b/service-t/src/com/android/server/net/NetworkStatsObservers.java
index 21cf351..cab29e3 100644
--- a/service-t/src/com/android/server/net/NetworkStatsObservers.java
+++ b/service-t/src/com/android/server/net/NetworkStatsObservers.java
@@ -32,7 +32,6 @@
import android.net.NetworkTemplate;
import android.net.netstats.IUsageCallback;
import android.os.Handler;
-import android.os.HandlerThread;
import android.os.IBinder;
import android.os.Looper;
import android.os.Message;
@@ -46,6 +45,7 @@
import com.android.internal.annotations.VisibleForTesting;
import com.android.net.module.util.PerUidCounter;
+import java.util.Objects;
import java.util.concurrent.atomic.AtomicInteger;
/**
@@ -78,8 +78,11 @@
// Sequence number of DataUsageRequests
private final AtomicInteger mNextDataUsageRequestId = new AtomicInteger();
- // Lazily instantiated when an observer is registered.
- private volatile Handler mHandler;
+ private final Handler mHandler;
+
+ NetworkStatsObservers(@NonNull Looper looper) {
+ mHandler = new Handler(Objects.requireNonNull(looper), mHandlerCallback);
+ }
/**
* Creates a wrapper that contains the caller context and a normalized request.
@@ -100,7 +103,7 @@
if (LOG) Log.d(TAG, "Registering observer for " + requestInfo);
mDataUsageRequestsPerUid.incrementCountOrThrow(callingUid);
- getHandler().sendMessage(mHandler.obtainMessage(MSG_REGISTER, requestInfo));
+ mHandler.sendMessage(mHandler.obtainMessage(MSG_REGISTER, requestInfo));
return request;
}
@@ -110,7 +113,7 @@
* <p>It will unregister the observer asynchronously, so it is safe to call from any thread.
*/
public void unregister(DataUsageRequest request, int callingUid) {
- getHandler().sendMessage(mHandler.obtainMessage(MSG_UNREGISTER, callingUid, 0 /* ignore */,
+ mHandler.sendMessage(mHandler.obtainMessage(MSG_UNREGISTER, callingUid, 0 /* ignore */,
request));
}
@@ -125,34 +128,10 @@
long currentTime) {
StatsContext statsContext = new StatsContext(xtSnapshot, uidSnapshot, activeIfaces,
activeUidIfaces, currentTime);
- getHandler().sendMessage(mHandler.obtainMessage(MSG_UPDATE_STATS, statsContext));
+ mHandler.sendMessage(mHandler.obtainMessage(MSG_UPDATE_STATS, statsContext));
}
- private Handler getHandler() {
- if (mHandler == null) {
- synchronized (this) {
- if (mHandler == null) {
- if (LOGV) Log.v(TAG, "Creating handler");
- mHandler = new Handler(getHandlerLooperLocked(), mHandlerCallback);
- }
- }
- }
- return mHandler;
- }
-
- @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();
- }
-
- private Handler.Callback mHandlerCallback = new Handler.Callback() {
+ private final Handler.Callback mHandlerCallback = new Handler.Callback() {
@Override
public boolean handleMessage(Message msg) {
switch (msg.what) {
diff --git a/service-t/src/com/android/server/net/NetworkStatsService.java b/service-t/src/com/android/server/net/NetworkStatsService.java
index 9684d18..64b17eb 100644
--- a/service-t/src/com/android/server/net/NetworkStatsService.java
+++ b/service-t/src/com/android/server/net/NetworkStatsService.java
@@ -593,7 +593,7 @@
INetd.Stub.asInterface((IBinder) context.getSystemService(Context.NETD_SERVICE)),
alarmManager, wakeLock, getDefaultClock(),
new DefaultNetworkStatsSettings(), new NetworkStatsFactory(context),
- new NetworkStatsObservers(), new Dependencies());
+ new Dependencies());
return service;
}
@@ -603,8 +603,7 @@
@VisibleForTesting
NetworkStatsService(Context context, INetd netd, AlarmManager alarmManager,
PowerManager.WakeLock wakeLock, Clock clock, NetworkStatsSettings settings,
- NetworkStatsFactory factory, NetworkStatsObservers statsObservers,
- @NonNull Dependencies deps) {
+ NetworkStatsFactory factory, @NonNull Dependencies deps) {
mContext = Objects.requireNonNull(context, "missing Context");
mNetd = Objects.requireNonNull(netd, "missing Netd");
mAlarmManager = Objects.requireNonNull(alarmManager, "missing AlarmManager");
@@ -612,7 +611,6 @@
mSettings = Objects.requireNonNull(settings, "missing NetworkStatsSettings");
mWakeLock = Objects.requireNonNull(wakeLock, "missing WakeLock");
mStatsFactory = Objects.requireNonNull(factory, "missing factory");
- mStatsObservers = Objects.requireNonNull(statsObservers, "missing NetworkStatsObservers");
mDeps = Objects.requireNonNull(deps, "missing Dependencies");
mStatsDir = mDeps.getOrCreateStatsDir();
if (!mStatsDir.exists()) {
@@ -622,6 +620,7 @@
final HandlerThread handlerThread = mDeps.makeHandlerThread();
handlerThread.start();
mHandler = new NetworkStatsHandler(handlerThread.getLooper());
+ mStatsObservers = new NetworkStatsObservers(handlerThread.getLooper());
mNetworkStatsSubscriptionsMonitor = deps.makeSubscriptionsMonitor(mContext,
(command) -> mHandler.post(command) , this);
mContentResolver = mContext.getContentResolver();
diff --git a/tests/unit/java/com/android/server/net/NetworkStatsObserversTest.java b/tests/unit/java/com/android/server/net/NetworkStatsObserversTest.java
index e62ac74..0bbc34c 100644
--- a/tests/unit/java/com/android/server/net/NetworkStatsObserversTest.java
+++ b/tests/unit/java/com/android/server/net/NetworkStatsObserversTest.java
@@ -47,7 +47,6 @@
import android.net.NetworkTemplate;
import android.os.HandlerThread;
import android.os.IBinder;
-import android.os.Looper;
import android.os.Process;
import android.os.UserHandle;
import android.telephony.TelephonyManager;
@@ -127,13 +126,7 @@
mObserverHandlerThread = new HandlerThread("NetworkStatsObserversTest");
mObserverHandlerThread.start();
- final Looper observerLooper = mObserverHandlerThread.getLooper();
- mStatsObservers = new NetworkStatsObservers() {
- @Override
- protected Looper getHandlerLooperLocked() {
- return observerLooper;
- }
- };
+ mStatsObservers = new NetworkStatsObservers(mObserverHandlerThread.getLooper());
mActiveIfaces = new ArrayMap<>();
mActiveUidIfaces = new ArrayMap<>();
diff --git a/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java b/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java
index 3ed51bc..3d7ad66 100644
--- a/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java
+++ b/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java
@@ -123,7 +123,6 @@
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;
@@ -293,7 +292,6 @@
private String mCompareStatsResult = null;
private @Mock Resources mResources;
private Boolean mIsDebuggable;
- private HandlerThread mObserverHandlerThread;
final TestDependencies mDeps = new TestDependencies();
private class MockContext extends BroadcastInterceptingContext {
@@ -377,21 +375,8 @@
powerManager.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, TAG);
mHandlerThread = new HandlerThread("NetworkStatsServiceTest-HandlerThread");
- // 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, statsObservers, mDeps);
+ mClock, mSettings, mStatsFactory, mDeps);
mElapsedRealtime = 0L;
@@ -589,10 +574,6 @@
mHandlerThread.quitSafely();
mHandlerThread.join();
}
- if (mObserverHandlerThread != null) {
- mObserverHandlerThread.quitSafely();
- mObserverHandlerThread.join();
- }
}
private void initWifiStats(NetworkStateSnapshot snapshot) throws Exception {