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.

This change also is cherry-picking cleanup made to NSS in
http://aosp/628368.

Bug: 123961098
Bug: 126245192
Bug: 120145746
Test: atest FrameworksNetTests
Change-Id: Ia687845888434c8ddd24bdf44b4c70dfe80e03f5
Merged-In: I57e117bb4e9efe491b19d6b5a479f2d58d1c58e6
This commit is contained in:
Varun Anand
2019-02-07 14:13:13 -08:00
parent 5a43a6735e
commit 687d8bd094
2 changed files with 29 additions and 32 deletions

View File

@@ -19,11 +19,13 @@ package android.net;
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 @@ interface INetworkStatsService {
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();

View File

@@ -82,7 +82,6 @@ import android.content.IntentFilter;
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;
@@ -161,9 +160,8 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
// Perform polling and persist all (FLAG_PERSIST_ALL).
private static final int MSG_PERFORM_POLL = 1;
private static final int MSG_UPDATE_IFACES = 2;
// Perform polling, persist network, and register the global alert again.
private static final int MSG_PERFORM_POLL_REGISTER_ALERT = 3;
private static final int MSG_PERFORM_POLL_REGISTER_ALERT = 2;
/** Flags to control detail level of poll event. */
private static final int FLAG_PERSIST_NETWORK = 0x1;
@@ -196,8 +194,6 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
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 +255,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
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 +266,10 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
@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();
@@ -371,10 +372,6 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
mHandlerCallback = callback;
}
public void bindConnectivityManager(IConnectivityManager connManager) {
mConnManager = checkNotNull(connManager, "missing IConnectivityManager");
}
public void systemReady() {
mSystemReady = true;
@@ -853,13 +850,17 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
}
@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);
}
@@ -1077,11 +1078,17 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
}
};
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();
}
@@ -1095,7 +1102,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
* {@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()");
@@ -1107,18 +1114,6 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
// 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();
@@ -1230,7 +1225,7 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
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);
@@ -1689,10 +1684,6 @@ public class NetworkStatsService extends INetworkStatsService.Stub {
mService.performPoll(FLAG_PERSIST_ALL);
return true;
}
case MSG_UPDATE_IFACES: {
mService.updateIfaces(null);
return true;
}
case MSG_PERFORM_POLL_REGISTER_ALERT: {
mService.performPoll(FLAG_PERSIST_NETWORK);
mService.registerGlobalAlert();