Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit d33cbc6f authored by Varun Anand's avatar Varun Anand
Browse files

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: Id1da446b54d95ee68ed14079107b1a10318bcf8b
Merged-In: I57e117bb4e9efe491b19d6b5a479f2d58d1c58e6
parent 222d1d04

core/java/android/net/IConnectivityManager.aidl

+0 −2
Original line number Diff line number Diff line
@@ -122,8 +122,6 @@ interface IConnectivityManager 


    LegacyVpnInfo getLegacyVpnInfo(int userId);



    VpnInfo[] getAllVpnInfo();



    boolean updateLockdownVpn();

    boolean isAlwaysOnVpnPackageSupported(int userId, String packageName);

    boolean setAlwaysOnVpnPackage(int userId, String packageName, boolean lockdown,

core/java/android/net/INetworkStatsService.aidl

+7 −1
Original line number Diff line number Diff line
@@ -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();

services/core/java/com/android/server/ConnectivityService.java

+16 −6
Original line number Diff line number Diff line
@@ -4096,12 +4096,14 @@ public class ConnectivityService extends IConnectivityManager.Stub 
    }



    /**

     * Return the information of all ongoing VPNs. This method is used by NetworkStatsService

     * and not available in ConnectivityManager.

     * Return the information of all ongoing VPNs.

     *

     * <p>This method is used to update NetworkStatsService.

     *

     * <p>Must be called on the handler thread.

     */

    @Override

    public VpnInfo[] getAllVpnInfo() {

        enforceConnectivityInternalPermission();

    private VpnInfo[] getAllVpnInfo() {

        ensureRunningOnConnectivityServiceThread();

        synchronized (mVpns) {

            if (mLockdownEnabled) {

                return new VpnInfo[0];
@@ -6397,6 +6399,7 @@ public class ConnectivityService extends IConnectivityManager.Stub 
     * Must be called on the handler thread.

     */

    private Network[] getDefaultNetworks() {

        ensureRunningOnConnectivityServiceThread();

        ArrayList<Network> defaultNetworks = new ArrayList<>();

        NetworkAgentInfo defaultNetwork = getDefaultNetwork();

        for (NetworkAgentInfo nai : mNetworkAgentInfos.values()) {
@@ -6412,8 +6415,15 @@ public class ConnectivityService extends IConnectivityManager.Stub 
     * properties tracked by NetworkStatsService on an active iface has changed.

     */

    private void notifyIfacesChangedForNetworkStats() {

        ensureRunningOnConnectivityServiceThread();

        String activeIface = null;

        LinkProperties activeLinkProperties = getActiveLinkProperties();

        if (activeLinkProperties != null) {

            activeIface = activeLinkProperties.getInterfaceName();

        }

        try {

            mStatsService.forceUpdateIfaces(getDefaultNetworks());

            mStatsService.forceUpdateIfaces(

                    getDefaultNetworks(), getAllVpnInfo(), getAllNetworkState(), activeIface);

        } catch (Exception ignored) {

        }

    }

services/core/java/com/android/server/net/NetworkStatsService.java

+21 −25
Original line number Diff line number Diff line
@@ -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;
@@ -195,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";
@@ -258,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. */
@@ -268,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();
@@ -375,10 +377,6 @@ public class NetworkStatsService extends INetworkStatsService.Stub { 
        mHandlerCallback = callback;

    }



    public void bindConnectivityManager(IConnectivityManager connManager) {

        mConnManager = checkNotNull(connManager, "missing IConnectivityManager");

    }



    public void systemReady() {

        mSystemReady = true;
@@ -857,13 +855,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);

        }
@@ -1127,11 +1129,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();

            }
@@ -1145,7 +1153,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()");
@@ -1157,18 +1165,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();
@@ -1280,7 +1276,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);

services/java/com/android/server/SystemServer.java

+0 −1
Original line number Diff line number Diff line
@@ -1235,7 +1235,6 @@ public final class SystemServer { 
                ServiceManager.addService(Context.CONNECTIVITY_SERVICE, connectivity,

                            /* allowIsolated= */ false,

                    DUMP_FLAG_PRIORITY_HIGH | DUMP_FLAG_PRIORITY_NORMAL);

                networkStats.bindConnectivityManager(connectivity);

                networkPolicy.bindConnectivityManager(connectivity);

            } catch (Throwable e) {

                reportWtf("starting Connectivity Service", e);