diff --git a/service-t/src/com/android/server/net/NetworkStatsObservers.java b/service-t/src/com/android/server/net/NetworkStatsObservers.java index 21cf3514cc..cab29e396e 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.NetworkStatsHistory; 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 android.util.SparseArray; 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 @@ class NetworkStatsObservers { // 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 @@ class NetworkStatsObservers { 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 @@ class NetworkStatsObservers { *

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 @@ class NetworkStatsObservers { 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 9684d1801c..64b17eb278 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 @@ public class NetworkStatsService extends INetworkStatsService.Stub { 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 @@ public class NetworkStatsService extends INetworkStatsService.Stub { @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 @@ public class NetworkStatsService extends INetworkStatsService.Stub { 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 @@ public class NetworkStatsService extends INetworkStatsService.Stub { 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 e62ac74429..0bbc34cad3 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.NetworkStatsAccess; 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 @@ public class NetworkStatsObserversTest { 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 3ed51bc01d..3d7ad66ace 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.DropBoxManager; 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 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { 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 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { 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 @@ public class NetworkStatsServiceTest extends NetworkStatsBaseTest { mHandlerThread.quitSafely(); mHandlerThread.join(); } - if (mObserverHandlerThread != null) { - mObserverHandlerThread.quitSafely(); - mObserverHandlerThread.join(); - } } private void initWifiStats(NetworkStateSnapshot snapshot) throws Exception {