diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index bfb51dadad..b9b590b9a0 100644 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -113,6 +113,7 @@ import static android.net.NetworkCapabilities.RES_ID_MATCH_ALL_RESERVATIONS; import static android.net.NetworkCapabilities.RES_ID_UNSET; import static android.net.NetworkCapabilities.TRANSPORT_CELLULAR; import static android.net.NetworkCapabilities.TRANSPORT_TEST; +import static android.net.NetworkCapabilities.TRANSPORT_THREAD; import static android.net.NetworkCapabilities.TRANSPORT_VPN; import static android.net.NetworkCapabilities.TRANSPORT_WIFI; import static android.net.NetworkRequest.Type.LISTEN_FOR_BEST; @@ -429,6 +430,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 @@ -4893,7 +4895,11 @@ public class ConnectivityService extends IConnectivityManager.Stub { // the destroyed flag is only just above the "current satisfier wins" // tie-breaker. But technically anything that affects scoring should rematch. rematchAllNetworksAndRequests(); - mHandler.postDelayed(() -> nai.disconnect(), timeoutMs); + if (mQueueNetworkAgentEventsInSystemServer) { + mHandler.postDelayed(() -> disconnectAndDestroyNetwork(nai), timeoutMs); + } else { + mHandler.postDelayed(() -> nai.disconnect(), timeoutMs); + } break; } } @@ -5323,12 +5329,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) { @@ -5443,15 +5449,18 @@ public class ConnectivityService extends IConnectivityManager.Stub { } @VisibleForTesting - protected static boolean shouldCreateNetworksImmediately(@NonNull NetworkCapabilities caps) { + protected boolean shouldCreateNetworksImmediately(@NonNull NetworkCapabilities caps) { // The feature of creating the networks immediately was slated for U, but race conditions // detected late required this was flagged off. - // TODO : enable this in a Mainline update or in V, and re-enable the test for this - // in NetworkAgentTest. - return caps.hasCapability(NET_CAPABILITY_LOCAL_NETWORK); + // TODO : remove when it's determined that the code is stable + return mQueueNetworkAgentEventsInSystemServer + // Local network agents for Thread used to not create networks immediately, + // but other local agents (tethering, P2P) require this to function. + || (caps.hasCapability(NET_CAPABILITY_LOCAL_NETWORK) + && !caps.hasTransport(TRANSPORT_THREAD)); } - private static boolean shouldCreateNativeNetwork(@NonNull NetworkAgentInfo nai, + private boolean shouldCreateNativeNetwork(@NonNull NetworkAgentInfo nai, @NonNull NetworkInfo.State state) { if (nai.isCreated()) return false; if (state == NetworkInfo.State.CONNECTED) return true; @@ -5508,6 +5517,11 @@ public class ConnectivityService extends IConnectivityManager.Stub { if (DBG) { log(nai.toShortString() + " disconnected, was satisfying " + nai.numNetworkRequests()); } + + if (mQueueNetworkAgentEventsInSystemServer) { + nai.disconnect(); + } + // Clear all notifications of this network. mNotifier.clearNotification(nai.network.getNetId()); // A network agent has disconnected. @@ -5651,16 +5665,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 +5682,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 +5855,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 +6155,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 +6224,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 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 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 +6602,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 offersToUpdate = new ArrayList<>(mNetworkOffers); for (final NetworkOfferInfo noi : offersToUpdate) { @@ -6896,19 +6947,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); @@ -6978,7 +7025,11 @@ public class ConnectivityService extends IConnectivityManager.Stub { final NetworkAgentInfo nai = getNetworkAgentInfoForNetwork((Network) msg.obj); if (nai == null) break; nai.onPreventAutomaticReconnect(); - nai.disconnect(); + if (mQueueNetworkAgentEventsInSystemServer) { + disconnectAndDestroyNetwork(nai); + } else { + nai.disconnect(); + } break; case EVENT_SET_VPN_NETWORK_PREFERENCE: handleSetVpnNetworkPreference((VpnNetworkPreferenceInfo) msg.obj); @@ -7368,12 +7419,12 @@ public class ConnectivityService extends IConnectivityManager.Stub { return new UnderlyingNetworkInfo[0]; } List 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 +7502,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 blockedUidRanges) { @@ -7503,11 +7554,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 +9122,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 mNetworkAgentInfos = new ArraySet<>(); // UID ranges for users that are currently blocked by VPNs. @@ -10439,7 +10493,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); @@ -10451,7 +10505,7 @@ public class ConnectivityService extends IConnectivityManager.Stub { mCarrierPrivilegeAuthenticator); updateCapabilities(nai.getScore(), nai, nc); } - } + }); } /** @@ -11208,7 +11262,11 @@ public class ConnectivityService extends IConnectivityManager.Stub { break; } } - nai.disconnect(); + if (mQueueNetworkAgentEventsInSystemServer) { + disconnectAndDestroyNetwork(nai); + } else { + nai.disconnect(); + } } private void handleLingerComplete(NetworkAgentInfo oldNetwork) { @@ -11368,7 +11426,7 @@ public class ConnectivityService extends IConnectivityManager.Stub { throw new IllegalStateException("No user is available"); } - for (final NetworkAgentInfo nai : mNetworkAgentInfos) { + forEachNetworkAgentInfo(nai -> { ArraySet allowedUidRanges = new ArraySet<>(); for (final UserHandle user : users) { final ArraySet restrictedUidRanges = @@ -11380,7 +11438,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 @@ -11602,9 +11660,7 @@ public class ConnectivityService extends IConnectivityManager.Stub { // Gather the list of all relevant agents. final ArrayList 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. @@ -11710,14 +11766,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 localInfoChangedAgents = null; - for (final NetworkAgentInfo nai : mNetworkAgentInfos) { - if (!nai.isLocalNetwork()) continue; + final ArrayList 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()) { @@ -11745,9 +11801,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 @@ -11798,17 +11853,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 @@ -11826,7 +11878,7 @@ public class ConnectivityService extends IConnectivityManager.Stub { teardownUnneededNetwork(nai); } } - } + }); } /** @@ -12215,7 +12267,9 @@ public class ConnectivityService extends IConnectivityManager.Stub { // This has to happen after matching the requests, because callbacks are just requests. notifyNetworkCallbacks(networkAgent, CALLBACK_PRECHECK); } else if (state == NetworkInfo.State.DISCONNECTED) { - networkAgent.disconnect(); + if (!mQueueNetworkAgentEventsInSystemServer) { + networkAgent.disconnect(); + } if (networkAgent.isVPN()) { updateVpnUids(networkAgent, networkAgent.networkCapabilities, null); } @@ -12339,7 +12393,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); @@ -12347,9 +12401,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); @@ -12358,7 +12410,7 @@ public class ConnectivityService extends IConnectivityManager.Stub { newBlockedState); } } - } + }); } @VisibleForTesting @@ -12447,11 +12499,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; } @@ -13342,15 +13394,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 @@ -13521,18 +13568,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)); }); } } @@ -14313,7 +14358,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) { @@ -14324,7 +14369,7 @@ public class ConnectivityService extends IConnectivityManager.Stub { mIngressRateLimit); } } - } + }); } private boolean canNetworkBeRateLimited(@NonNull final NetworkAgentInfo networkAgent) { diff --git a/tests/cts/net/src/android/net/cts/DscpPolicyTest.kt b/tests/cts/net/src/android/net/cts/DscpPolicyTest.kt index df4dab5544..d531e7abee 100644 --- a/tests/cts/net/src/android/net/cts/DscpPolicyTest.kt +++ b/tests/cts/net/src/android/net/cts/DscpPolicyTest.kt @@ -774,7 +774,9 @@ class DscpPolicyTest { runAsShell(MANAGE_TEST_NETWORKS) { agent.register() } // Without the fix, this will crash the system with SIGSEGV. agent.sendAddDscpPolicy(DscpPolicy.Builder(1, 1).build()) - agent.expectCallback() + // Will receive OnNetworkCreated first if the agent is created early. To avoid reading + // the flag here, use eventuallyExpect. + agent.eventuallyExpect() } } diff --git a/tests/cts/net/src/android/net/cts/NetworkAgentTest.kt b/tests/cts/net/src/android/net/cts/NetworkAgentTest.kt index 64dfcc83df..4703ac7f22 100644 --- a/tests/cts/net/src/android/net/cts/NetworkAgentTest.kt +++ b/tests/cts/net/src/android/net/cts/NetworkAgentTest.kt @@ -180,6 +180,10 @@ private const val TAG = "NetworkAgentTest" // without affecting the run time of successful runs. Thus, set a very high timeout. private const val DEFAULT_TIMEOUT_MS = 5000L +private const val QUEUE_NETWORK_AGENT_EVENTS_IN_SYSTEM_SERVER = + "queue_network_agent_events_in_system_server" + + // When waiting for a NetworkCallback to determine there was no timeout, waiting is the // only possible thing (the relevant handler is the one in the real ConnectivityService, // and then there is the Binder call), so have a short timeout for this as it will be @@ -204,12 +208,6 @@ private val REMOTE_ADDRESS = InetAddresses.parseNumericAddress("2001:db8::123") private val PREFIX = IpPrefix("2001:db8::/64") private val NEXTHOP = InetAddresses.parseNumericAddress("fe80::abcd") -// On T and below, the native network is only created when the agent connects. -// Starting in U, the native network was to be created as soon as the agent is registered, -// but this has been flagged off for now pending resolution of race conditions. -// TODO : enable this in a Mainline update or in V. -private const val SHOULD_CREATE_NETWORKS_IMMEDIATELY = false - @AppModeFull(reason = "Instant apps can't use NetworkAgent because it needs NETWORK_FACTORY'.") // NetworkAgent is updated as part of the connectivity module, and running NetworkAgent tests in MTS // for modules other than Connectivity does not provide much value. Only run them in connectivity @@ -235,6 +233,18 @@ class NetworkAgentTest { private var qosTestSocket: Closeable? = null // either Socket or DatagramSocket private val ifacesToCleanUp = mutableListOf() + // Unless the queuing in system server feature is chickened out, native networks are created + // immediately. Historically they would only created as they'd connect, which would force + // the code to apply link properties multiple times and suffer errors early on. Creating + // them early required that ordering between the client and the system server is guaranteed + // (at least to some extent), which has been done by moving the event queue from the client + // to the system server. When that feature is not chickened out, create networks immediately. + private val SHOULD_CREATE_NETWORKS_IMMEDIATELY + get() = mCM.isConnectivityServiceFeatureEnabledForTesting( + QUEUE_NETWORK_AGENT_EVENTS_IN_SYSTEM_SERVER + ) + + @Before fun setUp() { instrumentation.getUiAutomation().adoptShellPermissionIdentity() @@ -1689,16 +1699,17 @@ class NetworkAgentTest { // Connect a third network. Because network1 is awaiting replacement, network3 is preferred // as soon as it validates (until then, it is outscored by network1). - // The fact that the first events seen by matchAllCallback is the connection of network3 + // The fact that the first event seen by matchAllCallback is the connection of network3 // implicitly ensures that no callbacks are sent since network1 was lost. val (agent3, network3) = connectNetwork(lp = lp) - matchAllCallback.expectAvailableThenValidatedCallbacks(network3) - testCallback.expectAvailableDoubleValidatedCallbacks(network3) - sendAndExpectUdpPacket(network3, reader, iface) // As soon as the replacement arrives, network1 is disconnected. // Check that this happens before the replacement timeout (5 seconds) fires. + matchAllCallback.expectAvailableCallbacks(network3, validated = false) matchAllCallback.expect(network1, 2_000 /* timeoutMs */) + matchAllCallback.expectCaps(network3) { it.hasCapability(NET_CAPABILITY_VALIDATED) } + sendAndExpectUdpPacket(network3, reader, iface) + testCallback.expectAvailableDoubleValidatedCallbacks(network3) agent1.expectCallback() // Test lingering: @@ -1746,7 +1757,7 @@ class NetworkAgentTest { val callback = TestableNetworkCallback() requestNetwork(makeTestNetworkRequest(specifier = specifier6), callback) val agent6 = createNetworkAgent(specifier = specifier6) - val network6 = agent6.register() + agent6.register() if (SHOULD_CREATE_NETWORKS_IMMEDIATELY) { agent6.expectCallback() } else { @@ -1816,8 +1827,9 @@ class NetworkAgentTest { val (newWifiAgent, newWifiNetwork) = connectNetwork(TRANSPORT_WIFI) testCallback.expectAvailableCallbacks(newWifiNetwork, validated = true) - matchAllCallback.expectAvailableThenValidatedCallbacks(newWifiNetwork) + matchAllCallback.expectAvailableCallbacks(newWifiNetwork, validated = false) matchAllCallback.expect(wifiNetwork) + matchAllCallback.expectCaps(newWifiNetwork) { it.hasCapability(NET_CAPABILITY_VALIDATED) } wifiAgent.expectCallback() testCallback.expect(newWifiNetwork) diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java index 3eefa0f227..35a7fbd9c8 100755 --- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java @@ -3092,23 +3092,24 @@ public class ConnectivityServiceTest { generalCb.expectAvailableCallbacksUnvalidated(net2); if (expectLingering) { generalCb.expectLosing(net1); - } - generalCb.expectCaps(net2, c -> c.hasCapability(NET_CAPABILITY_VALIDATED)); - defaultCb.expectAvailableDoubleValidatedCallbacks(net2); - // Make sure cell 1 is unwanted immediately if the radio can't time share, but only - // after some delay if it can. - if (expectLingering) { + // Make sure cell 1 is unwanted immediately if the radio can't time share, but only + // after some delay if it can. + generalCb.expectCaps(net2, c -> c.hasCapability(NET_CAPABILITY_VALIDATED)); + defaultCb.expectAvailableDoubleValidatedCallbacks(net2); net1.assertNotDisconnected(TEST_CALLBACK_TIMEOUT_MS); // always incurs the timeout generalCb.assertNoCallback(); // assertNotDisconnected waited for TEST_CALLBACK_TIMEOUT_MS, so waiting for the // linger period gives TEST_CALLBACK_TIMEOUT_MS time for the event to process. net1.expectDisconnected(UNREASONABLY_LONG_ALARM_WAIT_MS); + generalCb.expect(LOST, net1); } else { net1.expectDisconnected(TEST_CALLBACK_TIMEOUT_MS); + net1.disconnect(); + generalCb.expect(LOST, net1); + generalCb.expectCaps(net2, c -> c.hasCapability(NET_CAPABILITY_VALIDATED)); + defaultCb.expectAvailableDoubleValidatedCallbacks(net2); } - net1.disconnect(); - generalCb.expect(LOST, net1); // Remove primary from net 2 net2.setScore(new NetworkScore.Builder().build());