Revert "Use Service handler thread for NetworkStatsObservers"
This reverts commit 8b1d824ad2e3dd3e14b87f2883d36f0b6a6bc213.
Reason for revert: b/332214348
Fix: 332214348
Change-Id: Iac8b9a11fb5b1c2e704eada62869c8c8552b3804
diff --git a/service-t/src/com/android/server/net/NetworkStatsObservers.java b/service-t/src/com/android/server/net/NetworkStatsObservers.java
index cab29e3..21cf351 100644
--- a/service-t/src/com/android/server/net/NetworkStatsObservers.java
+++ b/service-t/src/com/android/server/net/NetworkStatsObservers.java
@@ -32,6 +32,7 @@
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;
@@ -45,7 +46,6 @@
import com.android.internal.annotations.VisibleForTesting;
import com.android.net.module.util.PerUidCounter;
-import java.util.Objects;
import java.util.concurrent.atomic.AtomicInteger;
/**
@@ -78,11 +78,8 @@
// Sequence number of DataUsageRequests
private final AtomicInteger mNextDataUsageRequestId = new AtomicInteger();
- private final Handler mHandler;
-
- NetworkStatsObservers(@NonNull Looper looper) {
- mHandler = new Handler(Objects.requireNonNull(looper), mHandlerCallback);
- }
+ // Lazily instantiated when an observer is registered.
+ private volatile Handler mHandler;
/**
* Creates a wrapper that contains the caller context and a normalized request.
@@ -103,7 +100,7 @@
if (LOG) Log.d(TAG, "Registering observer for " + requestInfo);
mDataUsageRequestsPerUid.incrementCountOrThrow(callingUid);
- mHandler.sendMessage(mHandler.obtainMessage(MSG_REGISTER, requestInfo));
+ getHandler().sendMessage(mHandler.obtainMessage(MSG_REGISTER, requestInfo));
return request;
}
@@ -113,7 +110,7 @@
* <p>It will unregister the observer asynchronously, so it is safe to call from any thread.
*/
public void unregister(DataUsageRequest request, int callingUid) {
- mHandler.sendMessage(mHandler.obtainMessage(MSG_UNREGISTER, callingUid, 0 /* ignore */,
+ getHandler().sendMessage(mHandler.obtainMessage(MSG_UNREGISTER, callingUid, 0 /* ignore */,
request));
}
@@ -128,10 +125,34 @@
long currentTime) {
StatsContext statsContext = new StatsContext(xtSnapshot, uidSnapshot, activeIfaces,
activeUidIfaces, currentTime);
- mHandler.sendMessage(mHandler.obtainMessage(MSG_UPDATE_STATS, statsContext));
+ getHandler().sendMessage(mHandler.obtainMessage(MSG_UPDATE_STATS, statsContext));
}
- private final Handler.Callback mHandlerCallback = new Handler.Callback() {
+ 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() {
@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 64b17eb..9684d18 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 Dependencies());
+ new NetworkStatsObservers(), new Dependencies());
return service;
}
@@ -603,7 +603,8 @@
@VisibleForTesting
NetworkStatsService(Context context, INetd netd, AlarmManager alarmManager,
PowerManager.WakeLock wakeLock, Clock clock, NetworkStatsSettings settings,
- NetworkStatsFactory factory, @NonNull Dependencies deps) {
+ NetworkStatsFactory factory, NetworkStatsObservers statsObservers,
+ @NonNull Dependencies deps) {
mContext = Objects.requireNonNull(context, "missing Context");
mNetd = Objects.requireNonNull(netd, "missing Netd");
mAlarmManager = Objects.requireNonNull(alarmManager, "missing AlarmManager");
@@ -611,6 +612,7 @@
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()) {
@@ -620,7 +622,6 @@
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 0bbc34c..e62ac74 100644
--- a/tests/unit/java/com/android/server/net/NetworkStatsObserversTest.java
+++ b/tests/unit/java/com/android/server/net/NetworkStatsObserversTest.java
@@ -47,6 +47,7 @@
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;
@@ -126,7 +127,13 @@
mObserverHandlerThread = new HandlerThread("NetworkStatsObserversTest");
mObserverHandlerThread.start();
- mStatsObservers = new NetworkStatsObservers(mObserverHandlerThread.getLooper());
+ final Looper observerLooper = mObserverHandlerThread.getLooper();
+ mStatsObservers = new NetworkStatsObservers() {
+ @Override
+ protected Looper getHandlerLooperLocked() {
+ return observerLooper;
+ }
+ };
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 3d7ad66..3ed51bc 100644
--- a/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java
+++ b/tests/unit/java/com/android/server/net/NetworkStatsServiceTest.java
@@ -123,6 +123,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;
@@ -292,6 +293,7 @@
private String mCompareStatsResult = null;
private @Mock Resources mResources;
private Boolean mIsDebuggable;
+ private HandlerThread mObserverHandlerThread;
final TestDependencies mDeps = new TestDependencies();
private class MockContext extends BroadcastInterceptingContext {
@@ -375,8 +377,21 @@
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, mDeps);
+ mClock, mSettings, mStatsFactory, statsObservers, mDeps);
mElapsedRealtime = 0L;
@@ -574,6 +589,10 @@
mHandlerThread.quitSafely();
mHandlerThread.join();
}
+ if (mObserverHandlerThread != null) {
+ mObserverHandlerThread.quitSafely();
+ mObserverHandlerThread.join();
+ }
}
private void initWifiStats(NetworkStateSnapshot snapshot) throws Exception {