diff --git a/core/java/android/app/usage/NetworkStatsManager.java b/core/java/android/app/usage/NetworkStatsManager.java index 9c4a8f4fbe..5b98188300 100644 --- a/core/java/android/app/usage/NetworkStatsManager.java +++ b/core/java/android/app/usage/NetworkStatsManager.java @@ -526,15 +526,17 @@ public class NetworkStatsManager { } /** - * Registers a custom provider of {@link android.net.NetworkStats} to combine the network - * statistics that cannot be seen by the kernel to system. To unregister, invoke - * {@link NetworkStatsProviderCallback#unregister()}. + * Registers a custom provider of {@link android.net.NetworkStats} to provide network statistics + * to the system. To unregister, invoke {@link NetworkStatsProviderCallback#unregister()}. + * Note that no de-duplication of statistics between providers is performed, so each provider + * must only report network traffic that is not being reported by any other provider. * - * @param tag a human readable identifier of the custom network stats provider. - * @param provider a custom implementation of {@link AbstractNetworkStatsProvider} that needs to - * be registered to the system. + * @param tag a human readable identifier of the custom network stats provider. This is only + * used for debugging. + * @param provider the subclass of {@link AbstractNetworkStatsProvider} that needs to be + * registered to the system. * @return a {@link NetworkStatsProviderCallback}, which can be used to report events to the - * system. + * system or unregister the provider. * @hide */ @SystemApi diff --git a/services/core/java/com/android/server/net/NetworkStatsService.java b/services/core/java/com/android/server/net/NetworkStatsService.java index bde9ee24c7..0662400dbb 100644 --- a/services/core/java/com/android/server/net/NetworkStatsService.java +++ b/services/core/java/com/android/server/net/NetworkStatsService.java @@ -155,6 +155,8 @@ import java.util.Arrays; import java.util.HashSet; import java.util.List; import java.util.Objects; +import java.util.concurrent.Semaphore; +import java.util.concurrent.TimeUnit; /** * Collect and persist detailed network statistics, and provide this data to @@ -255,7 +257,6 @@ public class NetworkStatsService extends INetworkStatsService.Stub { } private final Object mStatsLock = new Object(); - private final Object mStatsProviderLock = new Object(); /** Set of currently active ifaces. */ @GuardedBy("mStatsLock") @@ -280,8 +281,11 @@ public class NetworkStatsService extends INetworkStatsService.Stub { private final DropBoxNonMonotonicObserver mNonMonotonicObserver = new DropBoxNonMonotonicObserver(); + private static final int MAX_STATS_PROVIDER_POLL_WAIT_TIME_MS = 100; private final RemoteCallbackList mStatsProviderCbList = new RemoteCallbackList<>(); + /** Semaphore used to wait for stats provider to respond to request stats update. */ + private final Semaphore mStatsProviderSem = new Semaphore(0, true); @GuardedBy("mStatsLock") private NetworkStatsRecorder mDevRecorder; @@ -1337,6 +1341,25 @@ public class NetworkStatsService extends INetworkStatsService.Stub { final boolean persistUid = (flags & FLAG_PERSIST_UID) != 0; final boolean persistForce = (flags & FLAG_PERSIST_FORCE) != 0; + // Request asynchronous stats update from all providers for next poll. And wait a bit of + // time to allow providers report-in given that normally binder call should be fast. + // TODO: request with a valid token. + Trace.traceBegin(TRACE_TAG_NETWORK, "provider.requestStatsUpdate"); + final int registeredCallbackCount = mStatsProviderCbList.getRegisteredCallbackCount(); + mStatsProviderSem.drainPermits(); + invokeForAllStatsProviderCallbacks((cb) -> cb.mProvider.requestStatsUpdate(0 /* unused */)); + try { + mStatsProviderSem.tryAcquire(registeredCallbackCount, + MAX_STATS_PROVIDER_POLL_WAIT_TIME_MS, TimeUnit.MILLISECONDS); + } catch (InterruptedException e) { + // Strictly speaking it's possible a provider happened to deliver between the timeout + // and the log, and that doesn't matter too much as this is just a debug log. + Log.d(TAG, "requestStatsUpdate - providers responded " + + mStatsProviderSem.availablePermits() + + "/" + registeredCallbackCount + " : " + e); + } + Trace.traceEnd(TRACE_TAG_NETWORK); + // TODO: consider marking "untrusted" times in historical stats final long currentTime = mClock.millis(); @@ -1374,10 +1397,6 @@ public class NetworkStatsService extends INetworkStatsService.Stub { performSampleLocked(); } - // request asynchronous stats update from all providers for next poll. - // TODO: request with a valid token. - invokeForAllStatsProviderCallbacks((cb) -> cb.mProvider.requestStatsUpdate(0 /* unused */)); - // finally, dispatch updated event to any listeners final Intent updatedIntent = new Intent(ACTION_NETWORK_STATS_UPDATED); updatedIntent.setFlags(Intent.FLAG_RECEIVER_REGISTERED_ONLY); @@ -1501,8 +1520,8 @@ public class NetworkStatsService extends INetworkStatsService.Stub { } @Override - public void setStatsProviderLimit(@NonNull String iface, long quota) { - Slog.v(TAG, "setStatsProviderLimit(" + iface + "," + quota + ")"); + public void setStatsProviderLimitAsync(@NonNull String iface, long quota) { + Slog.v(TAG, "setStatsProviderLimitAsync(" + iface + "," + quota + ")"); invokeForAllStatsProviderCallbacks((cb) -> cb.mProvider.setLimit(iface, quota)); } } @@ -1783,9 +1802,9 @@ public class NetworkStatsService extends INetworkStatsService.Stub { * {@code unregister()} of the returned callback. * * @param tag a human readable identifier of the custom network stats provider. - * @param provider the binder interface of - * {@link android.net.netstats.provider.AbstractNetworkStatsProvider} that - * needs to be registered to the system. + * @param provider the {@link INetworkStatsProvider} binder corresponding to the + * {@link android.net.netstats.provider.AbstractNetworkStatsProvider} to be + * registered. * * @return a binder interface of * {@link android.net.netstats.provider.NetworkStatsProviderCallback}, which can be @@ -1798,7 +1817,8 @@ public class NetworkStatsService extends INetworkStatsService.Stub { Objects.requireNonNull(tag, "tag is null"); try { NetworkStatsProviderCallbackImpl callback = new NetworkStatsProviderCallbackImpl( - tag, provider, mAlertObserver, mStatsProviderCbList); + tag, provider, mStatsProviderSem, mAlertObserver, + mStatsProviderCbList); mStatsProviderCbList.register(callback); Log.d(TAG, "registerNetworkStatsProvider from " + callback.mTag + " uid/pid=" + getCallingUid() + "/" + getCallingPid()); @@ -1823,7 +1843,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { private void invokeForAllStatsProviderCallbacks( @NonNull ThrowingConsumer task) { - synchronized (mStatsProviderCbList) { + synchronized (mStatsLock) { final int length = mStatsProviderCbList.beginBroadcast(); try { for (int i = 0; i < length; i++) { @@ -1844,25 +1864,30 @@ public class NetworkStatsService extends INetworkStatsService.Stub { private static class NetworkStatsProviderCallbackImpl extends INetworkStatsProviderCallback.Stub implements IBinder.DeathRecipient { @NonNull final String mTag; - @NonNull private final Object mProviderStatsLock = new Object(); + @NonNull final INetworkStatsProvider mProvider; + @NonNull private final Semaphore mSemaphore; @NonNull final INetworkManagementEventObserver mAlertObserver; @NonNull final RemoteCallbackList mStatsProviderCbList; + @NonNull private final Object mProviderStatsLock = new Object(); + @GuardedBy("mProviderStatsLock") - // STATS_PER_IFACE and STATS_PER_UID + // Track STATS_PER_IFACE and STATS_PER_UID separately. private final NetworkStats mIfaceStats = new NetworkStats(0L, 0); @GuardedBy("mProviderStatsLock") private final NetworkStats mUidStats = new NetworkStats(0L, 0); NetworkStatsProviderCallbackImpl( @NonNull String tag, @NonNull INetworkStatsProvider provider, + @NonNull Semaphore semaphore, @NonNull INetworkManagementEventObserver alertObserver, @NonNull RemoteCallbackList cbList) throws RemoteException { mTag = tag; mProvider = provider; mProvider.asBinder().linkToDeath(this, 0); + mSemaphore = semaphore; mAlertObserver = alertObserver; mStatsProviderCbList = cbList; } @@ -1881,7 +1906,8 @@ public class NetworkStatsService extends INetworkStatsService.Stub { default: throw new IllegalArgumentException("Invalid type: " + how); } - // Return a defensive copy instead of local reference. + // Callers might be able to mutate the returned object. Return a defensive copy + // instead of local reference. return stats.clone(); } } @@ -1895,6 +1921,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub { if (ifaceStats != null) mIfaceStats.combineAllValues(ifaceStats); if (uidStats != null) mUidStats.combineAllValues(uidStats); } + mSemaphore.release(); } @Override