Fix a tethering crash when a 464xlat upstream disconnects.
When a 464xlat upstream disconnects, onLinkPropertiesChanged is
called after onLost. This breaks an UpstreamNetworkMonitor
assumption that no callback will ever arrive after onLost.
Bug: 173068192
Fix: 185117377
Test: new unit test
Change-Id: I4ff1eca6d5ed1680ff716c71b683891c8a0e5a77
diff --git a/Tethering/src/com/android/networkstack/tethering/UpstreamNetworkMonitor.java b/Tethering/src/com/android/networkstack/tethering/UpstreamNetworkMonitor.java
index e39145b..f9af777 100644
--- a/Tethering/src/com/android/networkstack/tethering/UpstreamNetworkMonitor.java
+++ b/Tethering/src/com/android/networkstack/tethering/UpstreamNetworkMonitor.java
@@ -43,6 +43,7 @@
import android.util.SparseIntArray;
import androidx.annotation.NonNull;
+import androidx.annotation.Nullable;
import com.android.internal.annotations.VisibleForTesting;
import com.android.internal.util.StateMachine;
@@ -402,13 +403,20 @@
notifyTarget(EVENT_ON_CAPABILITIES, network);
}
- private void updateLinkProperties(Network network, LinkProperties newLp) {
+ private @Nullable UpstreamNetworkState updateLinkProperties(@NonNull Network network,
+ LinkProperties newLp) {
final UpstreamNetworkState prev = mNetworkMap.get(network);
if (prev == null || newLp.equals(prev.linkProperties)) {
// Ignore notifications about networks for which we have not yet
// received onAvailable() (should never happen) and any duplicate
// notifications (e.g. matching more than one of our callbacks).
- return;
+ //
+ // Also, it can happen that onLinkPropertiesChanged is called after
+ // onLost removed the state from mNetworkMap. This appears to be due
+ // to a bug in disconnectAndDestroyNetwork, which calls
+ // nai.clatd.update() after the onLost callbacks.
+ // TODO: fix the bug and make this method void.
+ return null;
}
if (VDBG) {
@@ -416,13 +424,17 @@
network, newLp));
}
- mNetworkMap.put(network, new UpstreamNetworkState(
- newLp, prev.networkCapabilities, network));
+ final UpstreamNetworkState ns = new UpstreamNetworkState(newLp, prev.networkCapabilities,
+ network);
+ mNetworkMap.put(network, ns);
+ return ns;
}
private void handleLinkProp(Network network, LinkProperties newLp) {
- updateLinkProperties(network, newLp);
- notifyTarget(EVENT_ON_LINKPROPERTIES, network);
+ final UpstreamNetworkState ns = updateLinkProperties(network, newLp);
+ if (ns != null) {
+ notifyTarget(EVENT_ON_LINKPROPERTIES, ns);
+ }
}
private void handleLost(Network network) {
diff --git a/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java
index 776298c..e042df4 100644
--- a/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java
+++ b/Tethering/tests/unit/src/com/android/networkstack/tethering/TetheringTest.java
@@ -1184,6 +1184,11 @@
assertTrue(mUpstreamNetworkMonitor.getCurrentPreferredUpstream().linkProperties
.hasIPv4Address());
+
+ // Check that the code does not crash if onLinkPropertiesChanged is received after onLost.
+ mobile.fakeDisconnect();
+ mobile.sendLinkProperties();
+ mLooper.dispatchAll();
}
private void runNcmTethering() {
