Add unit tests for race conditions in upstream selection. am: 6748e62ef2

Original change: https://android-review.googlesource.com/c/platform/packages/modules/Connectivity/+/1662399

Change-Id: Id2dcfdc1ec744e67f9f7877ea264bffdf1354b62
This commit is contained in:
Lorenzo Colitti
2021-04-06 03:59:19 +00:00
committed by Automerger Merge Worker
2 changed files with 81 additions and 17 deletions

View File

@@ -32,6 +32,8 @@ import android.os.Handler;
import android.os.UserHandle;
import android.util.ArrayMap;
import androidx.annotation.Nullable;
import java.util.Map;
import java.util.Objects;
@@ -58,6 +60,9 @@ import java.util.Objects;
* that state changes), this may become less important or unnecessary.
*/
public class TestConnectivityManager extends ConnectivityManager {
public static final boolean BROADCAST_FIRST = false;
public static final boolean CALLBACKS_FIRST = true;
final Map<NetworkCallback, NetworkRequestInfo> mAllCallbacks = new ArrayMap<>();
final Map<NetworkCallback, NetworkRequestInfo> mTrackingDefault = new ArrayMap<>();
final Map<NetworkCallback, NetworkRequestInfo> mListening = new ArrayMap<>();
@@ -151,14 +156,29 @@ public class TestConnectivityManager extends ConnectivityManager {
}
}
void makeDefaultNetwork(TestNetworkAgent agent) {
void makeDefaultNetwork(TestNetworkAgent agent, boolean order, @Nullable Runnable inBetween) {
if (Objects.equals(mDefaultNetwork, agent)) return;
final TestNetworkAgent formerDefault = mDefaultNetwork;
mDefaultNetwork = agent;
sendDefaultNetworkCallbacks(formerDefault, mDefaultNetwork);
sendDefaultNetworkBroadcasts(formerDefault, mDefaultNetwork);
if (order == CALLBACKS_FIRST) {
sendDefaultNetworkCallbacks(formerDefault, mDefaultNetwork);
if (inBetween != null) inBetween.run();
sendDefaultNetworkBroadcasts(formerDefault, mDefaultNetwork);
} else {
sendDefaultNetworkBroadcasts(formerDefault, mDefaultNetwork);
if (inBetween != null) inBetween.run();
sendDefaultNetworkCallbacks(formerDefault, mDefaultNetwork);
}
}
void makeDefaultNetwork(TestNetworkAgent agent, boolean order) {
makeDefaultNetwork(agent, order, null /* inBetween */);
}
void makeDefaultNetwork(TestNetworkAgent agent) {
makeDefaultNetwork(agent, BROADCAST_FIRST, null /* inBetween */);
}
@Override
@@ -308,6 +328,7 @@ public class TestConnectivityManager extends ConnectivityManager {
networkId, copy(networkCapabilities)));
nri.handler.post(() -> cb.onLinkPropertiesChanged(networkId, copy(linkProperties)));
}
// mTrackingDefault will be updated if/when the caller calls makeDefaultNetwork
}
public void fakeDisconnect() {
@@ -320,6 +341,7 @@ public class TestConnectivityManager extends ConnectivityManager {
for (NetworkCallback cb : cm.mListening.keySet()) {
cb.onLost(networkId);
}
// mTrackingDefault will be updated if/when the caller calls makeDefaultNetwork
}
public void sendLinkProperties() {

View File

@@ -59,6 +59,8 @@ import static android.telephony.SubscriptionManager.INVALID_SUBSCRIPTION_ID;
import static com.android.net.module.util.Inet4AddressUtils.inet4AddressToIntHTH;
import static com.android.net.module.util.Inet4AddressUtils.intToInet4AddressHTH;
import static com.android.networkstack.tethering.TestConnectivityManager.BROADCAST_FIRST;
import static com.android.networkstack.tethering.TestConnectivityManager.CALLBACKS_FIRST;
import static com.android.networkstack.tethering.Tethering.UserRestrictionActionListener;
import static com.android.networkstack.tethering.TetheringNotificationUpdater.DOWNSTREAM_NONE;
import static com.android.networkstack.tethering.UpstreamNetworkMonitor.EVENT_ON_CAPABILITIES;
@@ -1107,22 +1109,49 @@ public class TetheringTest {
// Pretend cellular connected and expect the upstream to be set.
TestNetworkAgent mobile = new TestNetworkAgent(mCm, buildMobileDualStackUpstreamState());
mobile.fakeConnect();
mCm.makeDefaultNetwork(mobile);
mCm.makeDefaultNetwork(mobile, BROADCAST_FIRST);
mLooper.dispatchAll();
inOrder.verify(mUpstreamNetworkMonitor).setCurrentUpstream(mobile.networkId);
// Switch upstreams a few times.
// TODO: there may be a race where if the effects of the CONNECTIVITY_ACTION happen before
// UpstreamNetworkMonitor gets onCapabilitiesChanged on CALLBACK_DEFAULT_INTERNET, the
// upstream does not change. Extend TestConnectivityManager to simulate this condition and
// write a test for this.
TestNetworkAgent wifi = new TestNetworkAgent(mCm, buildWifiUpstreamState());
wifi.fakeConnect();
mCm.makeDefaultNetwork(wifi);
mCm.makeDefaultNetwork(wifi, BROADCAST_FIRST);
mLooper.dispatchAll();
inOrder.verify(mUpstreamNetworkMonitor).setCurrentUpstream(wifi.networkId);
mCm.makeDefaultNetwork(mobile);
final Runnable doDispatchAll = () -> mLooper.dispatchAll();
// There is a race where if Tethering and UpstreamNetworkMonitor process the
// CONNECTIVITY_ACTION before UpstreamNetworkMonitor gets onCapabilitiesChanged on
// CALLBACK_DEFAULT_INTERNET, the upstream does not change.
// Simulate this by telling TestConnectivityManager to call mLooper.dispatchAll() between
// sending the CONNECTIVITY_ACTION and sending the callbacks.
// The switch to wifi just above shows that if the CONNECTIVITY_ACTION and the callbacks
// happen close enough together that the tethering code sees both, the code behaves as
// expected.
mCm.makeDefaultNetwork(mobile, BROADCAST_FIRST, doDispatchAll);
mLooper.dispatchAll();
inOrder.verify(mUpstreamNetworkMonitor, never()).setCurrentUpstream(any()); // BUG
mCm.makeDefaultNetwork(wifi, BROADCAST_FIRST, doDispatchAll);
mLooper.dispatchAll();
inOrder.verify(mUpstreamNetworkMonitor).setCurrentUpstream(mobile.networkId); // BUG
// If the broadcast immediately follows the callbacks, the code behaves as expected.
// In this case nothing happens because the upstream is already set to cellular and
// remaining on cellular is correct.
mCm.makeDefaultNetwork(mobile, CALLBACKS_FIRST);
mLooper.dispatchAll();
inOrder.verify(mUpstreamNetworkMonitor, never()).setCurrentUpstream(any());
mCm.makeDefaultNetwork(wifi, CALLBACKS_FIRST);
mLooper.dispatchAll();
inOrder.verify(mUpstreamNetworkMonitor).setCurrentUpstream(wifi.networkId);
// If the broadcast happens after the callbacks have been processed, the code also behaves
// correctly.
mCm.makeDefaultNetwork(mobile, CALLBACKS_FIRST, doDispatchAll);
mLooper.dispatchAll();
inOrder.verify(mUpstreamNetworkMonitor).setCurrentUpstream(mobile.networkId);
@@ -1132,27 +1161,40 @@ public class TetheringTest {
inOrder.verify(mUpstreamNetworkMonitor, never()).setCurrentUpstream(any());
// Lose and regain upstream.
// As above, if broadcasts are processed before the callbacks, upstream is not updated.
assertTrue(mUpstreamNetworkMonitor.getCurrentPreferredUpstream().linkProperties
.hasIPv4Address());
mobile.fakeDisconnect();
mCm.makeDefaultNetwork(null);
mCm.makeDefaultNetwork(null, BROADCAST_FIRST, doDispatchAll);
mLooper.dispatchAll();
inOrder.verify(mUpstreamNetworkMonitor).setCurrentUpstream(null);
mobile.fakeDisconnect();
mLooper.dispatchAll();
inOrder.verify(mUpstreamNetworkMonitor, never()).setCurrentUpstream(any()); // BUG
mobile = new TestNetworkAgent(mCm, buildMobile464xlatUpstreamState());
mobile.fakeConnect();
mCm.makeDefaultNetwork(mobile);
mCm.makeDefaultNetwork(mobile, BROADCAST_FIRST, doDispatchAll);
mLooper.dispatchAll();
inOrder.verify(mUpstreamNetworkMonitor).setCurrentUpstream(mobile.networkId);
inOrder.verify(mUpstreamNetworkMonitor).setCurrentUpstream(null); // BUG
// Check the IP addresses to ensure that the upstream is indeed not the same as the previous
// mobile upstream, even though the netId is (unrealistically) the same.
assertFalse(mUpstreamNetworkMonitor.getCurrentPreferredUpstream().linkProperties
.hasIPv4Address());
// Lose and regain upstream again, but this time send events in an order that works.
mCm.makeDefaultNetwork(null, CALLBACKS_FIRST, doDispatchAll);
mobile.fakeDisconnect();
mCm.makeDefaultNetwork(null);
mLooper.dispatchAll();
inOrder.verify(mUpstreamNetworkMonitor).setCurrentUpstream(null);
inOrder.verify(mUpstreamNetworkMonitor, never()).setCurrentUpstream(any()); // BUG
mobile = new TestNetworkAgent(mCm, buildMobileDualStackUpstreamState());
mobile.fakeConnect();
mCm.makeDefaultNetwork(mobile, CALLBACKS_FIRST, doDispatchAll);
mLooper.dispatchAll();
inOrder.verify(mUpstreamNetworkMonitor).setCurrentUpstream(mobile.networkId);
assertTrue(mUpstreamNetworkMonitor.getCurrentPreferredUpstream().linkProperties
.hasIPv4Address());
}
private void runNcmTethering() {