From 5ea8ed1787d92bce3b07c049e9917cbd6bfc39ef Mon Sep 17 00:00:00 2001 From: Roshan Pius Date: Tue, 12 May 2020 10:01:42 -0700 Subject: ClientModeImpl: Ignore NETWORK_CONNECTION_EVENT while disconnecting If we've triggered a disconnect, ignore any NETWORK_CONNECTION_EVENTs (triggered if the fw roams just before that). Since we will anyway get a NETWORK_DISCONNECTION_EVENT just after that, reacting to the transient event is not useful. Also, ensure that we explicitly unregister any previous network agent active when creating new one. This is just a failsafe since the fix above should anyway take care of the scenario in this particular bug. Bug: 151067137 Test: atest com.android.server.wifi Test: ACTS presubmit tests Change-Id: I9657c4775922888623794dd749a42378d553ebee Merged-In: I9657c4775922888623794dd749a42378d553ebee --- .../com/android/server/wifi/ClientModeImpl.java | 11 ++++++ .../android/server/wifi/ClientModeImplTest.java | 42 ++++++++++++++++++++++ 2 files changed, 53 insertions(+) diff --git a/service/java/com/android/server/wifi/ClientModeImpl.java b/service/java/com/android/server/wifi/ClientModeImpl.java index 879b98d13..71a8cc613 100644 --- a/service/java/com/android/server/wifi/ClientModeImpl.java +++ b/service/java/com/android/server/wifi/ClientModeImpl.java @@ -4590,28 +4590,33 @@ public class ClientModeImpl extends StateMachine { @Override public void onStartSocketKeepalive(int slot, @NonNull Duration interval, @NonNull KeepalivePacketData packet) { + if (this != mNetworkAgent) return; ClientModeImpl.this.sendMessage( CMD_START_IP_PACKET_OFFLOAD, slot, (int) interval.getSeconds(), packet); } @Override public void onStopSocketKeepalive(int slot) { + if (this != mNetworkAgent) return; ClientModeImpl.this.sendMessage(CMD_STOP_IP_PACKET_OFFLOAD, slot); } @Override public void onAddKeepalivePacketFilter(int slot, @NonNull KeepalivePacketData packet) { + if (this != mNetworkAgent) return; ClientModeImpl.this.sendMessage( CMD_ADD_KEEPALIVE_PACKET_FILTER_TO_APF, slot, 0, packet); } @Override public void onRemoveKeepalivePacketFilter(int slot) { + if (this != mNetworkAgent) return; ClientModeImpl.this.sendMessage(CMD_REMOVE_KEEPALIVE_PACKET_FILTER_FROM_APF, slot); } @Override public void onSignalStrengthThresholdsUpdated(@NonNull int[] thresholds) { + if (this != mNetworkAgent) return; // 0. If there are no thresholds, or if the thresholds are invalid, // stop RSSI monitoring. // 1. Tell the hardware to start RSSI monitoring here, possibly adding MIN_VALUE and @@ -4725,6 +4730,7 @@ public class ClientModeImpl extends StateMachine { // This should never happen. if (mNetworkAgent != null) { Log.wtf(TAG, "mNetworkAgent is not null: " + mNetworkAgent); + mNetworkAgent.unregister(); } mNetworkAgent = new WifiNetworkAgent(mContext, getHandler().getLooper(), "WifiNetworkAgent", nc, mLinkProperties, 60, naConfig, @@ -5568,6 +5574,11 @@ public class ClientModeImpl extends StateMachine { log("Ignore CMD_DISCONNECT when already disconnecting."); } break; + case WifiMonitor.NETWORK_CONNECTION_EVENT: + if (mVerboseLoggingEnabled) { + log("Ignore NETWORK_CONNECTION_EVENT when already disconnecting."); + } + break; case CMD_DISCONNECTING_WATCHDOG_TIMER: if (mDisconnectingWatchdogCount == message.arg1) { if (mVerboseLoggingEnabled) log("disconnecting watchdog! -> disconnect"); diff --git a/tests/wifitests/src/com/android/server/wifi/ClientModeImplTest.java b/tests/wifitests/src/com/android/server/wifi/ClientModeImplTest.java index 727922e83..53218f790 100644 --- a/tests/wifitests/src/com/android/server/wifi/ClientModeImplTest.java +++ b/tests/wifitests/src/com/android/server/wifi/ClientModeImplTest.java @@ -2454,6 +2454,19 @@ public class ClientModeImplTest extends WifiBaseTest { mLooper.dispatchAll(); } + private void expectUnregisterNetworkAgent() { + // We cannot just use a mock object here because mWifiNetworkAgent is private to CMI. + // TODO (b/134538181): consider exposing WifiNetworkAgent and using mocks. + ArgumentCaptor messageCaptor = ArgumentCaptor.forClass(Message.class); + mLooper.dispatchAll(); + verify(mNetworkAgentHandler).handleMessage(messageCaptor.capture()); + Message message = messageCaptor.getValue(); + assertNotNull(message); + assertEquals(NetworkAgent.EVENT_NETWORK_INFO_CHANGED, message.what); + NetworkInfo networkInfo = (NetworkInfo) message.obj; + assertEquals(NetworkInfo.DetailedState.DISCONNECTED, networkInfo.getDetailedState()); + } + private void expectNetworkAgentUpdateCapabilities( Consumer networkCapabilitiesChecker) { // We cannot just use a mock object here because mWifiNetworkAgent is private to CMI. @@ -5088,4 +5101,33 @@ public class ClientModeImplTest extends WifiBaseTest { verify(mWifiNative, never()).removeNetworkCachedData(anyInt()); } + @Test + public void testIpReachabilityLostAndRoamEventsRace() throws Exception { + connect(); + expectRegisterNetworkAgent((agentConfig) -> { }, (cap) -> { }); + reset(mNetworkAgentHandler); + + // Trigger ip reachibility loss and ensure we trigger a disconnect & we're in + // "DisconnectingState" + mCmi.sendMessage(ClientModeImpl.CMD_IP_REACHABILITY_LOST); + mLooper.dispatchAll(); + verify(mWifiNative).disconnect(any()); + assertEquals("DisconnectingState", getCurrentState().getName()); + + // Now send a network connection (indicating a roam) event before we get the disconnect + // event. + mCmi.sendMessage(WifiMonitor.NETWORK_CONNECTION_EVENT, 0, 0, sBSSID); + mLooper.dispatchAll(); + // ensure that we ignored the transient roam while we're disconnecting. + assertEquals("DisconnectingState", getCurrentState().getName()); + verifyNoMoreInteractions(mNetworkAgentHandler); + + // Now send the disconnect event and ensure that we transition to "DisconnectedState". + mCmi.sendMessage(WifiMonitor.NETWORK_DISCONNECTION_EVENT, 0, 0, sBSSID); + mLooper.dispatchAll(); + assertEquals("DisconnectedState", getCurrentState().getName()); + expectUnregisterNetworkAgent(); + + verifyNoMoreInteractions(mNetworkAgentHandler); + } } -- cgit v1.2.3