LineageOS/android_packages_modules_Connectivity
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Use CopyOnWriteArrayList to store list of NetworkStatsProviderCb

Browse Source 
In NetworkStatsService, mStatsLock will be held when
iterating through the provider list. This is to protect the list
from concurrent broadcast that triggered by
NetworkPolicyManagerService. This is not good since the binder
call is oneway, it does not make sense to block every
access to the providers.

This change also remove unuse variable and reduce verbose log.

Test: atest FrameworksNetTests TetheringTests
Bug: 150418178
Change-Id: If74e9f2ea597a0d5ae4668c3358bc687f342bbb5
This commit is contained in:
junyulai
2020-04-29 18:25:32 +08:00
committed by Junyu Lai
parent 87ba308377
commit a6880d0037
1 changed files with 19 additions and 30 deletions
Download Patch File Download Diff File Expand all files Collapse all files

43
services/core/java/com/android/server/net/NetworkStatsService.java
View File

@@ -123,7 +123,6 @@ import android.os.Looper;
import android.os.Message;
import android.os.Messenger;
import android.os.PowerManager;
import android.os.RemoteCallbackList;
import android.os.RemoteException;
import android.os.SystemClock;
import android.os.Trace;
@@ -166,6 +165,7 @@ import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import java.util.Objects;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.Executor;
import java.util.concurrent.Semaphore;
import java.util.concurrent.TimeUnit;
@@ -230,12 +230,6 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
private static final String PREFIX_UID = "uid";
private static final String PREFIX_UID_TAG = "uid_tag";
/**
* Virtual network interface for video telephony. This is for VT data usage counting purpose.
*/
// TODO: Remove this after no one is using it.
public static final String VT_INTERFACE = NetworkStats.IFACE_VT;
/**
* Settings that can be changed externally.
*/
@@ -308,8 +302,8 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
new DropBoxNonMonotonicObserver();
private static final int MAX_STATS_PROVIDER_POLL_WAIT_TIME_MS = 100;
private final RemoteCallbackList<NetworkStatsProviderCallbackImpl> mStatsProviderCbList =
new RemoteCallbackList<>();
private final CopyOnWriteArrayList<NetworkStatsProviderCallbackImpl> mStatsProviderCbList =
new CopyOnWriteArrayList<>();
/** Semaphore used to wait for stats provider to respond to request stats update. */
private final Semaphore mStatsProviderSem = new Semaphore(0, true);
@@ -1463,10 +1457,14 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
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.
// time to allow providers report-in given that normally binder call should be fast. Note
// that size of list might be changed because addition/removing at the same time. For
// addition, the stats of the missed provider can only be collected in next poll;
// for removal, wait might take up to MAX_STATS_PROVIDER_POLL_WAIT_TIME_MS
// once that happened.
// TODO: request with a valid token.
Trace.traceBegin(TRACE_TAG_NETWORK, "provider.requestStatsUpdate");
final int registeredCallbackCount = mStatsProviderCbList.getRegisteredCallbackCount();
final int registeredCallbackCount = mStatsProviderCbList.size();
mStatsProviderSem.drainPermits();
invokeForAllStatsProviderCallbacks(
(cb) -> cb.mProvider.onRequestStatsUpdate(0 /* unused */));
@@ -1643,7 +1641,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
@Override
public void setStatsProviderLimitAsync(@NonNull String iface, long quota) {
Slog.v(TAG, "setStatsProviderLimitAsync(" + iface + "," + quota + ")");
if (LOGV) Slog.v(TAG, "setStatsProviderLimitAsync(" + iface + "," + quota + ")");
invokeForAllStatsProviderCallbacks((cb) -> cb.mProvider.onSetLimit(iface, quota));
}
}
@@ -1934,7 +1932,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
NetworkStatsProviderCallbackImpl callback = new NetworkStatsProviderCallbackImpl(
tag, provider, mStatsProviderSem, mAlertObserver,
mStatsProviderCbList);
mStatsProviderCbList.register(callback);
mStatsProviderCbList.add(callback);
Log.d(TAG, "registerNetworkStatsProvider from " + callback.mTag + " uid/pid="
+ getCallingUid() + "/" + getCallingPid());
return callback;
@@ -1958,22 +1956,13 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
private void invokeForAllStatsProviderCallbacks(
@NonNull ThrowingConsumer<NetworkStatsProviderCallbackImpl, RemoteException> task) {
synchronized (mStatsLock) {
final int length = mStatsProviderCbList.beginBroadcast();
try {
for (int i = 0; i < length; i++) {
final NetworkStatsProviderCallbackImpl cb =
mStatsProviderCbList.getBroadcastItem(i);
for (final NetworkStatsProviderCallbackImpl cb : mStatsProviderCbList) {
try {
task.accept(cb);
} catch (RemoteException e) {
Log.e(TAG, "Fail to broadcast to provider: " + cb.mTag, e);
}
}
} finally {
mStatsProviderCbList.finishBroadcast();
}
}
}
private static class NetworkStatsProviderCallbackImpl extends INetworkStatsProviderCallback.Stub
@@ -1983,7 +1972,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
@NonNull final INetworkStatsProvider mProvider;
@NonNull private final Semaphore mSemaphore;
@NonNull final INetworkManagementEventObserver mAlertObserver;
@NonNull final RemoteCallbackList<NetworkStatsProviderCallbackImpl> mStatsProviderCbList;
@NonNull final CopyOnWriteArrayList<NetworkStatsProviderCallbackImpl> mStatsProviderCbList;
@NonNull private final Object mProviderStatsLock = new Object();
@@ -1997,7 +1986,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
@NonNull String tag, @NonNull INetworkStatsProvider provider,
@NonNull Semaphore semaphore,
@NonNull INetworkManagementEventObserver alertObserver,
@NonNull RemoteCallbackList<NetworkStatsProviderCallbackImpl> cbList)
@NonNull CopyOnWriteArrayList<NetworkStatsProviderCallbackImpl> cbList)
throws RemoteException {
mTag = tag;
mProvider = provider;
@@ -2054,13 +2043,13 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
@Override
public void binderDied() {
Log.d(TAG, mTag + ": binderDied");
mStatsProviderCbList.unregister(this);
mStatsProviderCbList.remove(this);
}
@Override
public void unregister() {
Log.d(TAG, mTag + ": unregister");
mStatsProviderCbList.unregister(this);
mStatsProviderCbList.remove(this);
}
}