Don't send onLinkPropertiesChanged after onLost for 464xlat.
Currently, when a network that uses 464xlat is torn down, NetworkCallbacks will receive onLinkPropertiesChanged after onLost. This is confusing and incorrect. The incorrect callback is sent because handleLinkProperties checks that the netId of the agent still exists, not that the NetworkAgent is still registered. This is normally correct, because the NetworkAgent is removed from mNetworkAgentInfos and the netId are removed from mNetworkForNetId by the same method, disconnectAndDestroyNetwork. In this specific case it's not correct, because the call to handleUpdateLinkProperties is from disconnectAndDestroyNetwork itself via nai.clatd.update and calls Nat464Xlat#stop. No other callers of handleUpdateLinkProperties are affected because: - EVENT_NETWORK_PROPERTIES_CHANGED is called only by maybeHandleNetworkAgentMessage, which first checks that the NetworkAgent is registered. - handlePrivateDnsSettingsChanged only looks at registered NetworkAgents (it loops over mNetworkAgentInfos). - handlePrivateDnsValidationUpdate, handleNat64PrefixEvent and handleCapportApiDataUpdate call getNetworkAgentInfoForNetId, which will correctly determine that the agent is no longer registered, since they run on the handler thread and thus cannot run at the same time as disconnectAndDestroyNetwork. The existing code contains a check for the netId being current. This is intended to ensure that an update from a NetworkAgent cannot affect another agent with the same Network. This extra check is not necessary, because code running on the handler thread can never observe a NetworkAgent in mNetworkAgentInfos unless mNetworkForNetId maps that NetworkAgent's Network to that NetworkAgent. This is because mNetworkForNetId is updated by the same methods as mNetworkAgentInfos, and those updates occur on the handler thread. So all code on the handler thread will see those two as consistent. Bug: 176496580 Test: atest FrameworksNetTests CtsNetTestCases HostsideVpnTests Change-Id: I944f4c6ad36206bdccd85a6ea7ef71324a29c685
This commit is contained in:
@@ -8544,13 +8544,8 @@ public class ConnectivityServiceTest {
|
||||
// Disconnect the network. clat is stopped and the network is destroyed.
|
||||
mCellNetworkAgent.disconnect();
|
||||
networkCallback.expectCallback(CallbackEntry.LOST, mCellNetworkAgent);
|
||||
// TODO: delete this spurious onLinkPropertiesChanged callback.
|
||||
networkCallback.expectLinkPropertiesThat(mCellNetworkAgent,
|
||||
lp -> lp.getStackedLinks().isEmpty());
|
||||
networkCallback.assertNoCallback();
|
||||
verify(mMockNetd).clatdStop(MOBILE_IFNAME);
|
||||
verify(mMockNetd).networkRemoveInterface(cellNetId, CLAT_MOBILE_IFNAME);
|
||||
assertRoutesRemoved(cellNetId, stackedDefault);
|
||||
verify(mMockNetd).idletimerRemoveInterface(eq(MOBILE_IFNAME), anyInt(),
|
||||
eq(Integer.toString(TRANSPORT_CELLULAR)));
|
||||
verify(mMockNetd).networkDestroy(cellNetId);
|
||||
|
||||
Reference in New Issue
Block a user