Always disconnect agents immediately.
Currently, various codepaths in ConnectivityService disconnect
networks using NetworkAgentInfo#disconnect. This posts a message
to the NetworkAgent to disconnect and also posts a message to
ConnectivityService to call disconnectAndDestroyNetwork, which
performs all cleanup in ConnectivityService. These two messages
race and the order is non-deterministic.
Instead, always disconnect using disconnectAndDestroyNetwork,
and have disconnectAndDestroyNetwork post the message to the
agent to disconnect.
This fixes a bug where if wifi uses unregisterAfterReplacement
twice in quick succession, when the third agent connects it
doesn't work because the interface is still being used by the
second network.
Also remove the import of IllegalArgumentException to keep the
linter happy (java.lang.* never needs to be imported).
Bug: 286649301
Test: covered by existing tests
Change-Id: Ie01f5589d6839ac6db25f0ba98fc929fbb5b0a96
diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java
index 447dbf4..39a500c 100755
--- a/service/src/com/android/server/ConnectivityService.java
+++ b/service/src/com/android/server/ConnectivityService.java
@@ -318,7 +318,6 @@
import java.io.InterruptedIOException;
import java.io.PrintWriter;
import java.io.Writer;
-import java.lang.IllegalArgumentException;
import java.net.Inet4Address;
import java.net.InetAddress;
import java.net.InetSocketAddress;
@@ -4010,7 +4009,7 @@
// 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);
+ mHandler.postDelayed(() -> disconnectAndDestroyNetwork(nai), timeoutMs);
break;
}
}
@@ -4609,6 +4608,9 @@
if (DBG) {
log(nai.toShortString() + " disconnected, was satisfying " + nai.numNetworkRequests());
}
+
+ nai.disconnect();
+
// Clear all notifications of this network.
mNotifier.clearNotification(nai.network.getNetId());
// A network agent has disconnected.
@@ -5893,7 +5895,7 @@
final NetworkAgentInfo nai = getNetworkAgentInfoForNetwork((Network) msg.obj);
if (nai == null) break;
nai.onPreventAutomaticReconnect();
- nai.disconnect();
+ disconnectAndDestroyNetwork(nai);
break;
case EVENT_SET_VPN_NETWORK_PREFERENCE:
handleSetVpnNetworkPreference((VpnNetworkPreferenceInfo) msg.obj);
@@ -9038,7 +9040,7 @@
break;
}
}
- nai.disconnect();
+ disconnectAndDestroyNetwork(nai);
}
private void handleLingerComplete(NetworkAgentInfo oldNetwork) {
@@ -9580,7 +9582,10 @@
updateLegacyTypeTrackerAndVpnLockdownForRematch(changes, nais);
// Tear down all unneeded networks.
- for (NetworkAgentInfo nai : mNetworkAgentInfos) {
+ // Iterate in reverse order because teardownUnneededNetwork removes the nai from
+ // mNetworkAgentInfos.
+ for (int i = mNetworkAgentInfos.size() - 1; i >= 0; i--) {
+ final NetworkAgentInfo nai = mNetworkAgentInfos.valueAt(i);
if (unneeded(nai, UnneededFor.TEARDOWN)) {
if (nai.getInactivityExpiry() > 0) {
// This network has active linger timers and no requests, but is not
@@ -9963,7 +9968,6 @@
// This has to happen after matching the requests, because callbacks are just requests.
notifyNetworkCallbacks(networkAgent, ConnectivityManager.CALLBACK_PRECHECK);
} else if (state == NetworkInfo.State.DISCONNECTED) {
- networkAgent.disconnect();
if (networkAgent.isVPN()) {
updateVpnUids(networkAgent, networkAgent.networkCapabilities, null);
}