Remove ConnectivityManager and its usages from NetworkStatsService.
NSS needed it for getting VpnInfo[], NetworkState[] and
activeLinkProperties which it used to query via ConnectivityManager.
For VpnInfo[], this was racy as NSS may ignore intermediate changes to a
VPN's underlying networks. See http://b/123961098 for more context.
It may also lead to deadlocks b/w ConnectivityService and
NetworkStatsService. See http://b/126245192 for more info.
This change will ensure that NSS is never contending on any of
ConnectivityService locks.
Bug: 123961098
Bug: 126245192
Bug: 120145746
Test: atest FrameworksNetTests
Change-Id: I57e117bb4e9efe491b19d6b5a479f2d58d1c58e6
diff --git a/core/java/android/net/INetworkStatsService.aidl b/core/java/android/net/INetworkStatsService.aidl
index 8e6f272..148b25d 100644
--- a/core/java/android/net/INetworkStatsService.aidl
+++ b/core/java/android/net/INetworkStatsService.aidl
@@ -19,11 +19,13 @@
import android.net.DataUsageRequest;
import android.net.INetworkStatsSession;
import android.net.Network;
+import android.net.NetworkState;
import android.net.NetworkStats;
import android.net.NetworkStatsHistory;
import android.net.NetworkTemplate;
import android.os.IBinder;
import android.os.Messenger;
+import com.android.internal.net.VpnInfo;
/** {@hide} */
interface INetworkStatsService {
@@ -58,7 +60,11 @@
void incrementOperationCount(int uid, int tag, int operationCount);
/** Force update of ifaces. */
- void forceUpdateIfaces(in Network[] defaultNetworks);
+ void forceUpdateIfaces(
+ in Network[] defaultNetworks,
+ in VpnInfo[] vpnArray,
+ in NetworkState[] networkStates,
+ in String activeIface);
/** Force update of statistics. */
void forceUpdate();
diff --git a/services/core/java/com/android/server/net/NetworkStatsService.java b/services/core/java/com/android/server/net/NetworkStatsService.java
index bffd60b..205ddb0 100644
--- a/services/core/java/com/android/server/net/NetworkStatsService.java
+++ b/services/core/java/com/android/server/net/NetworkStatsService.java
@@ -82,7 +82,6 @@
import android.content.pm.ApplicationInfo;
import android.content.pm.PackageManager;
import android.net.DataUsageRequest;
-import android.net.IConnectivityManager;
import android.net.INetworkManagementEventObserver;
import android.net.INetworkStatsService;
import android.net.INetworkStatsSession;
@@ -196,8 +195,6 @@
private final boolean mUseBpfTrafficStats;
- private IConnectivityManager mConnManager;
-
@VisibleForTesting
public static final String ACTION_NETWORK_STATS_POLL =
"com.android.server.action.NETWORK_STATS_POLL";
@@ -259,6 +256,7 @@
private final ArrayMap<String, NetworkIdentitySet> mActiveUidIfaces = new ArrayMap<>();
/** Current default active iface. */
+ @GuardedBy("mStatsLock")
private String mActiveIface;
/** Set of any ifaces associated with mobile networks since boot. */
@@ -269,6 +267,10 @@
@GuardedBy("mStatsLock")
private Network[] mDefaultNetworks = new Network[0];
+ /** Set containing info about active VPNs and their underlying networks. */
+ @GuardedBy("mStatsLock")
+ private VpnInfo[] mVpnInfos = new VpnInfo[0];
+
private final DropBoxNonMonotonicObserver mNonMonotonicObserver =
new DropBoxNonMonotonicObserver();
@@ -382,10 +384,6 @@
mHandlerCallback = callback;
}
- public void bindConnectivityManager(IConnectivityManager connManager) {
- mConnManager = checkNotNull(connManager, "missing IConnectivityManager");
- }
-
public void systemReady() {
mSystemReady = true;
@@ -864,13 +862,17 @@
}
@Override
- public void forceUpdateIfaces(Network[] defaultNetworks) {
+ public void forceUpdateIfaces(
+ Network[] defaultNetworks,
+ VpnInfo[] vpnArray,
+ NetworkState[] networkStates,
+ String activeIface) {
mContext.enforceCallingOrSelfPermission(READ_NETWORK_USAGE_HISTORY, TAG);
assertBandwidthControlEnabled();
final long token = Binder.clearCallingIdentity();
try {
- updateIfaces(defaultNetworks);
+ updateIfaces(defaultNetworks, vpnArray, networkStates, activeIface);
} finally {
Binder.restoreCallingIdentity(token);
}
@@ -1134,11 +1136,17 @@
}
};
- private void updateIfaces(Network[] defaultNetworks) {
+ private void updateIfaces(
+ Network[] defaultNetworks,
+ VpnInfo[] vpnArray,
+ NetworkState[] networkStates,
+ String activeIface) {
synchronized (mStatsLock) {
mWakeLock.acquire();
try {
- updateIfacesLocked(defaultNetworks);
+ mVpnInfos = vpnArray;
+ mActiveIface = activeIface;
+ updateIfacesLocked(defaultNetworks, networkStates);
} finally {
mWakeLock.release();
}
@@ -1152,7 +1160,7 @@
* {@link NetworkIdentitySet}.
*/
@GuardedBy("mStatsLock")
- private void updateIfacesLocked(Network[] defaultNetworks) {
+ private void updateIfacesLocked(Network[] defaultNetworks, NetworkState[] states) {
if (!mSystemReady) return;
if (LOGV) Slog.v(TAG, "updateIfacesLocked()");
@@ -1164,18 +1172,6 @@
// will be persisted during next alarm poll event.
performPollLocked(FLAG_PERSIST_NETWORK);
- final NetworkState[] states;
- final LinkProperties activeLink;
- try {
- states = mConnManager.getAllNetworkState();
- activeLink = mConnManager.getActiveLinkProperties();
- } catch (RemoteException e) {
- // ignored; service lives in system_server
- return;
- }
-
- mActiveIface = activeLink != null ? activeLink.getInterfaceName() : null;
-
// Rebuild active interfaces based on connected networks
mActiveIfaces.clear();
mActiveUidIfaces.clear();
@@ -1287,7 +1283,7 @@
Trace.traceEnd(TRACE_TAG_NETWORK);
// For per-UID stats, pass the VPN info so VPN traffic is reattributed to responsible apps.
- VpnInfo[] vpnArray = mConnManager.getAllVpnInfo();
+ VpnInfo[] vpnArray = mVpnInfos;
Trace.traceBegin(TRACE_TAG_NETWORK, "recordUid");
mUidRecorder.recordSnapshotLocked(uidSnapshot, mActiveUidIfaces, vpnArray, currentTime);
Trace.traceEnd(TRACE_TAG_NETWORK);
