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

2 files changed, 53 insertions, 0 deletions

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<Message> 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<NetworkCapabilities> 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);
+    }
 }