Make iterating over mNetworkAgentInfos safer.

This CL provides convenience methods to iterate over all
networks.
- forEachNetworkAgentInfo takes a consumer and runs it on all
  networks in mNetworkAgentInfos. The consumer is allowed to
  remove any number of networks. This is needed because some
  loops over mNetworkAgentInfos result in disconnecting networks,
  which removes them from the set.
- anyNetworkAgentInfo takes a predicate and returns true iff it
  returns true on any element in mNetworkAgentInfos. The
  predicate is not allowed to modify the set in any way.

Bug: 288149251
Test: simple refactor, covered by existing unit tests
Change-Id: I5d8b38503895d33abc2d96daaa1c96372befce64
This commit is contained in:
Chalard Jean 2025-03-18 19:53:35 +09:00
commit ede14bde88

View file

@ -429,6 +429,7 @@ import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.BiConsumer;
import java.util.function.Consumer;
import java.util.function.Predicate;
/**
* @hide
@ -5323,12 +5324,12 @@ public class ConnectivityService extends IConnectivityManager.Stub {
private void handlePrivateDnsSettingsChanged() {
final PrivateDnsConfig cfg = mDnsManager.getPrivateDnsConfig();
for (NetworkAgentInfo nai : mNetworkAgentInfos) {
forEachNetworkAgentInfo(nai -> {
handlePerNetworkPrivateDnsConfig(nai, cfg);
if (networkRequiresPrivateDnsValidation(nai)) {
handleUpdateLinkProperties(nai, new LinkProperties(nai.linkProperties));
}
}
});
}
private void handlePerNetworkPrivateDnsConfig(NetworkAgentInfo nai, PrivateDnsConfig cfg) {
@ -5651,16 +5652,16 @@ public class ConnectivityService extends IConnectivityManager.Stub {
private void maybeDisableForwardRulesForDisconnectingNai(
@NonNull final NetworkAgentInfo disconnecting, final boolean sendCallbacks) {
// Step 1 : maybe this network was the upstream for one or more local networks.
for (final NetworkAgentInfo local : mNetworkAgentInfos) {
if (!local.isLocalNetwork()) continue;
forEachNetworkAgentInfo(local -> {
if (!local.isLocalNetwork()) return; // return@forEach
final NetworkRequest selector = local.localNetworkConfig.getUpstreamSelector();
if (null == selector) continue;
if (null == selector) return; // return@forEach
final NetworkRequestInfo nri = mNetworkRequests.get(selector);
// null == nri can happen while disconnecting a network, because destroyNetwork() is
// called after removing all associated NRIs from mNetworkRequests.
if (null == nri) continue;
if (null == nri) return; // return@forEach
final NetworkAgentInfo satisfier = nri.getSatisfier();
if (disconnecting != satisfier) continue;
if (disconnecting != satisfier) return; // return@forEach
removeLocalNetworkUpstream(local, disconnecting);
// Set the satisfier to null immediately so that the LOCAL_NETWORK_CHANGED callback
// correctly contains null as an upstream.
@ -5668,7 +5669,7 @@ public class ConnectivityService extends IConnectivityManager.Stub {
nri.setSatisfier(null, null);
notifyNetworkCallbacks(local, CALLBACK_LOCAL_NETWORK_INFO_CHANGED);
}
}
});
// Step 2 : maybe this is a local network that had an upstream.
if (!disconnecting.isLocalNetwork()) return;
@ -5841,12 +5842,12 @@ public class ConnectivityService extends IConnectivityManager.Stub {
mNetworkRequests.put(req, nri);
// TODO: Consider update signal strength for other types.
if (req.isListen()) {
for (final NetworkAgentInfo network : mNetworkAgentInfos) {
forEachNetworkAgentInfo(network -> {
if (req.networkCapabilities.hasSignalStrength()
&& network.satisfiesImmutableCapabilitiesOf(req)) {
updateSignalStrengthThresholds(network, "REGISTER", req);
}
}
});
} else if (req.isRequest() && mNetworkRequestStateStatsMetrics != null) {
mNetworkRequestStateStatsMetrics.onNetworkRequestReceived(req);
}
@ -6141,13 +6142,13 @@ public class ConnectivityService extends IConnectivityManager.Stub {
private void removeListenRequestFromNetworks(@NonNull final NetworkRequest req) {
// listens don't have a singular affected Network. Check all networks to see
// if this listen request applies and remove it.
for (final NetworkAgentInfo nai : mNetworkAgentInfos) {
forEachNetworkAgentInfo(nai -> {
nai.removeRequest(req.requestId);
if (req.networkCapabilities.hasSignalStrength()
&& nai.satisfiesImmutableCapabilitiesOf(req)) {
updateSignalStrengthThresholds(nai, "RELEASE", req);
}
}
});
}
/**
@ -6210,6 +6211,43 @@ public class ConnectivityService extends IConnectivityManager.Stub {
}
}
/**
* Perform the specified operation on all networks.
*
* This method will run |op| exactly once for each network currently registered at the
* time it is called, even if |op| adds or removes networks.
*
* @param op the operation to perform. The operation is allowed to disconnect any number of
* networks.
*/
private void forEachNetworkAgentInfo(final Consumer<NetworkAgentInfo> op) {
// Create a copy instead of iterating over the set so |op| is allowed to disconnect any
// number of networks (which removes it from mNetworkAgentInfos). The copy is cheap
// because there are at most a handful of NetworkAgents connected at any given time.
final NetworkAgentInfo[] nais = new NetworkAgentInfo[mNetworkAgentInfos.size()];
mNetworkAgentInfos.toArray(nais);
for (NetworkAgentInfo nai : nais) {
op.accept(nai);
}
}
/**
* Check whether the specified condition is true for any network.
*
* This method will stop evaluating as soon as the condition returns true for any network.
* The order of iteration is not contractual.
*
* @param condition the condition to verify. This method must not modify the set of networks in
* any way.
* @return whether {@code condition} returned true for any network
*/
private boolean anyNetworkAgentInfo(final Predicate<NetworkAgentInfo> condition) {
for (int i = mNetworkAgentInfos.size() - 1; i >= 0; i--) {
if (condition.test(mNetworkAgentInfos.valueAt(i))) return true;
}
return false;
}
private RequestInfoPerUidCounter getRequestCounter(NetworkRequestInfo nri) {
return hasAnyPermissionOf(mContext,
nri.mPid, nri.mUid, NetworkStack.PERMISSION_MAINLINE_NETWORK_STACK)
@ -6551,14 +6589,14 @@ public class ConnectivityService extends IConnectivityManager.Stub {
ensureRunningOnConnectivityServiceThread();
// Agent info scores and offer scores depend on whether cells yields to bad wifi.
final boolean avoidBadWifi = avoidBadWifi();
for (final NetworkAgentInfo nai : mNetworkAgentInfos) {
forEachNetworkAgentInfo(nai -> {
nai.updateScoreForNetworkAgentUpdate();
if (avoidBadWifi) {
// If the device is now avoiding bad wifi, remove notifications that might have
// been put up when the device didn't.
mNotifier.clearNotification(nai.network.getNetId(), NotificationType.LOST_INTERNET);
}
}
});
// UpdateOfferScore will update mNetworkOffers inline, so make a copy first.
final ArrayList<NetworkOfferInfo> offersToUpdate = new ArrayList<>(mNetworkOffers);
for (final NetworkOfferInfo noi : offersToUpdate) {
@ -6896,19 +6934,15 @@ public class ConnectivityService extends IConnectivityManager.Stub {
final Network underpinnedNetwork = ki.getUnderpinnedNetwork();
final Network network = ki.getNetwork();
boolean networkFound = false;
boolean underpinnedNetworkFound = false;
for (NetworkAgentInfo n : mNetworkAgentInfos) {
if (n.network.equals(network)) networkFound = true;
if (n.everConnected() && n.network.equals(underpinnedNetwork)) {
underpinnedNetworkFound = true;
}
}
final boolean networkFound =
anyNetworkAgentInfo(n -> n.network.equals(network));
// If the network no longer exists, then the keepalive should have been
// cleaned up already. There is no point trying to resume keepalives.
if (!networkFound) return;
final boolean underpinnedNetworkFound = anyNetworkAgentInfo(
n -> n.everConnected() && n.network.equals(underpinnedNetwork));
if (underpinnedNetworkFound) {
mKeepaliveTracker.handleMonitorAutomaticKeepalive(ki,
underpinnedNetwork.netId);
@ -7368,12 +7402,12 @@ public class ConnectivityService extends IConnectivityManager.Stub {
return new UnderlyingNetworkInfo[0];
}
List<UnderlyingNetworkInfo> infoList = new ArrayList<>();
for (NetworkAgentInfo nai : mNetworkAgentInfos) {
forEachNetworkAgentInfo(nai -> {
UnderlyingNetworkInfo info = createVpnInfo(nai);
if (info != null) {
infoList.add(info);
}
}
});
return infoList.toArray(new UnderlyingNetworkInfo[infoList.size()]);
}
@ -7451,11 +7485,11 @@ public class ConnectivityService extends IConnectivityManager.Stub {
*/
private void propagateUnderlyingNetworkCapabilities(Network updatedNetwork) {
ensureRunningOnConnectivityServiceThread();
for (NetworkAgentInfo nai : mNetworkAgentInfos) {
forEachNetworkAgentInfo(nai -> {
if (updatedNetwork == null || hasUnderlyingNetwork(nai, updatedNetwork)) {
updateCapabilitiesForNetwork(nai);
}
}
});
}
private boolean isUidBlockedByVpn(int uid, List<UidRange> blockedUidRanges) {
@ -7503,11 +7537,11 @@ public class ConnectivityService extends IConnectivityManager.Stub {
mPermissionMonitor.updateVpnLockdownUidRanges(requireVpn, ranges);
}
for (final NetworkAgentInfo nai : mNetworkAgentInfos) {
forEachNetworkAgentInfo(nai -> {
final boolean curMetered = nai.networkCapabilities.isMetered();
maybeNotifyNetworkBlocked(nai, curMetered, curMetered,
mVpnBlockedUidRanges, newVpnBlockedUidRanges);
}
});
mVpnBlockedUidRanges = newVpnBlockedUidRanges;
}
@ -9071,6 +9105,9 @@ public class ConnectivityService extends IConnectivityManager.Stub {
// Tracks all NetworkAgents that are currently registered.
// NOTE: Only should be accessed on ConnectivityServiceThread, except dump().
// Code iterating over this set is recommended to use forAllNetworkAgentInfos(), which allows
// code within the loop to disconnect networks during iteration without causing null pointer or
// OOB exceptions.
private final ArraySet<NetworkAgentInfo> mNetworkAgentInfos = new ArraySet<>();
// UID ranges for users that are currently blocked by VPNs.
@ -10445,7 +10482,7 @@ public class ConnectivityService extends IConnectivityManager.Stub {
// A NetworkAgent's allowedUids may need to be updated if the app has lost
// carrier config
for (final NetworkAgentInfo nai : mNetworkAgentInfos) {
forEachNetworkAgentInfo(nai -> {
if (nai.networkCapabilities.getAllowedUidsNoCopy().contains(uid)
&& getSubscriptionIdFromNetworkCaps(nai.networkCapabilities) == subId) {
final NetworkCapabilities nc = new NetworkCapabilities(nai.networkCapabilities);
@ -10457,7 +10494,7 @@ public class ConnectivityService extends IConnectivityManager.Stub {
mCarrierPrivilegeAuthenticator);
updateCapabilities(nai.getScore(), nai, nc);
}
}
});
}
/**
@ -11374,7 +11411,7 @@ public class ConnectivityService extends IConnectivityManager.Stub {
throw new IllegalStateException("No user is available");
}
for (final NetworkAgentInfo nai : mNetworkAgentInfos) {
forEachNetworkAgentInfo(nai -> {
ArraySet<UidRange> allowedUidRanges = new ArraySet<>();
for (final UserHandle user : users) {
final ArraySet<UidRange> restrictedUidRanges =
@ -11386,7 +11423,7 @@ public class ConnectivityService extends IConnectivityManager.Stub {
final UidRangeParcel[] rangesParcel = toUidRangeStableParcels(allowedUidRanges);
configs.add(new NativeUidRangeConfig(
nai.network.netId, rangesParcel, 0 /* subPriority */));
}
});
// The netd API replaces the previous configs with the current configs.
// Thus, for network disconnection or preference removal, no need to
@ -11608,9 +11645,7 @@ public class ConnectivityService extends IConnectivityManager.Stub {
// Gather the list of all relevant agents.
final ArrayList<NetworkAgentInfo> nais = new ArrayList<>();
for (final NetworkAgentInfo nai : mNetworkAgentInfos) {
nais.add(nai);
}
forEachNetworkAgentInfo(nai -> nais.add(nai));
for (final NetworkRequestInfo nri : networkRequests) {
// Non-multilayer listen requests can be ignored.
@ -11716,14 +11751,14 @@ public class ConnectivityService extends IConnectivityManager.Stub {
// Don't send CALLBACK_LOCAL_NETWORK_INFO_CHANGED yet though : they should be sent after
// onAvailable so clients know what network the change is about. Store such changes in
// an array that's only allocated if necessary (because it's almost never necessary).
ArrayList<NetworkAgentInfo> localInfoChangedAgents = null;
for (final NetworkAgentInfo nai : mNetworkAgentInfos) {
if (!nai.isLocalNetwork()) continue;
final ArrayList<NetworkAgentInfo> localInfoChangedAgents = new ArrayList<>();
forEachNetworkAgentInfo(nai -> {
if (!nai.isLocalNetwork()) return; // return@forEach
final NetworkRequest nr = nai.localNetworkConfig.getUpstreamSelector();
if (null == nr) continue; // No upstream for this local network
if (null == nr) return; // return@forEach, no upstream for this local network
final NetworkRequestInfo nri = mNetworkRequests.get(nr);
final NetworkReassignment.RequestReassignment change = changes.getReassignment(nri);
if (null == change) continue; // No change in upstreams for this network
if (null == change) return; // return@forEach, no change in upstreams for this network
final String fromIface = nai.linkProperties.getInterfaceName();
if (!hasSameInterfaceName(change.mOldNetwork, change.mNewNetwork)
|| change.mOldNetwork.isDestroyed()) {
@ -11751,9 +11786,8 @@ public class ConnectivityService extends IConnectivityManager.Stub {
loge("Can't update forwarding rules", e);
}
}
if (null == localInfoChangedAgents) localInfoChangedAgents = new ArrayList<>();
localInfoChangedAgents.add(nai);
}
});
// Notify requested networks are available after the default net is switched, but
// before LegacyTypeTracker sends legacy broadcasts
@ -11804,17 +11838,14 @@ public class ConnectivityService extends IConnectivityManager.Stub {
}
// Send LOCAL_NETWORK_INFO_CHANGED callbacks now that onAvailable and onLost have been sent.
if (null != localInfoChangedAgents) {
for (final NetworkAgentInfo nai : localInfoChangedAgents) {
notifyNetworkCallbacks(nai,
CALLBACK_LOCAL_NETWORK_INFO_CHANGED);
}
for (final NetworkAgentInfo nai : localInfoChangedAgents) {
notifyNetworkCallbacks(nai, CALLBACK_LOCAL_NETWORK_INFO_CHANGED);
}
updateLegacyTypeTrackerAndVpnLockdownForRematch(changes, nais);
// Tear down all unneeded networks.
for (NetworkAgentInfo nai : mNetworkAgentInfos) {
forEachNetworkAgentInfo(nai -> {
if (unneeded(nai, UnneededFor.TEARDOWN)) {
if (nai.getInactivityExpiry() > 0) {
// This network has active linger timers and no requests, but is not
@ -11832,7 +11863,7 @@ public class ConnectivityService extends IConnectivityManager.Stub {
teardownUnneededNetwork(nai);
}
}
}
});
}
/**
@ -12345,7 +12376,7 @@ public class ConnectivityService extends IConnectivityManager.Stub {
* @param blockedReasons The reasons for why an uid is blocked.
*/
private void maybeNotifyNetworkBlockedForNewState(int uid, @BlockedReason int blockedReasons) {
for (final NetworkAgentInfo nai : mNetworkAgentInfos) {
forEachNetworkAgentInfo(nai -> {
final boolean metered = nai.networkCapabilities.isMetered();
final boolean vpnBlocked = isUidBlockedByVpn(uid, mVpnBlockedUidRanges);
@ -12353,9 +12384,7 @@ public class ConnectivityService extends IConnectivityManager.Stub {
uid, mUidBlockedReasons.get(uid, BLOCKED_REASON_NONE), metered, vpnBlocked);
final int newBlockedState =
getBlockedState(uid, blockedReasons, metered, vpnBlocked);
if (oldBlockedState == newBlockedState) {
continue;
}
if (oldBlockedState == newBlockedState) return; // return@forEach
for (int i = 0; i < nai.numNetworkRequests(); i++) {
NetworkRequest nr = nai.requestAt(i);
NetworkRequestInfo nri = mNetworkRequests.get(nr);
@ -12364,7 +12393,7 @@ public class ConnectivityService extends IConnectivityManager.Stub {
newBlockedState);
}
}
}
});
}
@VisibleForTesting
@ -12453,11 +12482,11 @@ public class ConnectivityService extends IConnectivityManager.Stub {
activeNetIds.add(nri.getSatisfier().network().netId);
}
}
for (NetworkAgentInfo nai : mNetworkAgentInfos) {
forEachNetworkAgentInfo(nai -> {
if (activeNetIds.contains(nai.network().netId) || nai.isVPN()) {
defaultNetworks.add(nai.network);
}
}
});
return defaultNetworks;
}
@ -13348,15 +13377,10 @@ public class ConnectivityService extends IConnectivityManager.Stub {
}
private boolean ownsVpnRunningOverNetwork(int uid, Network network) {
for (NetworkAgentInfo virtual : mNetworkAgentInfos) {
if (virtual.propagateUnderlyingCapabilities()
&& virtual.networkCapabilities.getOwnerUid() == uid
&& CollectionUtils.contains(virtual.declaredUnderlyingNetworks, network)) {
return true;
}
}
return false;
return anyNetworkAgentInfo(virtual ->
virtual.propagateUnderlyingCapabilities()
&& virtual.networkCapabilities.getOwnerUid() == uid
&& CollectionUtils.contains(virtual.declaredUnderlyingNetworks, network));
}
@CheckResult
@ -13527,18 +13551,16 @@ public class ConnectivityService extends IConnectivityManager.Stub {
@Override
public void onInterfaceLinkStateChanged(@NonNull String iface, boolean up) {
mHandler.post(() -> {
for (NetworkAgentInfo nai : mNetworkAgentInfos) {
nai.clatd.handleInterfaceLinkStateChanged(iface, up);
}
forEachNetworkAgentInfo(nai ->
nai.clatd.handleInterfaceLinkStateChanged(iface, up));
});
}
@Override
public void onInterfaceRemoved(@NonNull String iface) {
mHandler.post(() -> {
for (NetworkAgentInfo nai : mNetworkAgentInfos) {
nai.clatd.handleInterfaceRemoved(iface);
}
forEachNetworkAgentInfo(nai ->
nai.clatd.handleInterfaceRemoved(iface));
});
}
}
@ -14319,7 +14341,7 @@ public class ConnectivityService extends IConnectivityManager.Stub {
final long oldIngressRateLimit = mIngressRateLimit;
mIngressRateLimit = ConnectivitySettingsManager.getIngressRateLimitInBytesPerSecond(
mContext);
for (final NetworkAgentInfo networkAgent : mNetworkAgentInfos) {
forEachNetworkAgentInfo(networkAgent -> {
if (canNetworkBeRateLimited(networkAgent)) {
// If rate limit has previously been enabled, remove the old limit first.
if (oldIngressRateLimit >= 0) {
@ -14330,7 +14352,7 @@ public class ConnectivityService extends IConnectivityManager.Stub {
mIngressRateLimit);
}
}
}
});
}
private boolean canNetworkBeRateLimited(@NonNull final NetworkAgentInfo networkAgent) {