WifiNetworkFactory: Stop caching CMM instance
CMM instance creation / re-use should be controlled centrally by
ActiveModeWarden. Caching CMM instance locally in WifiNetworkFactory (when a new
request comes in while procesing another request) can lead to the device
connecting to the same bssid via 2 CMM.
Bug: 148935691
Test: atest com.android.server.wifi
Change-Id: Ia12ec0c32685b50026b415cd195540c8ec402544
diff --git a/service/java/com/android/server/wifi/ActiveModeWarden.java b/service/java/com/android/server/wifi/ActiveModeWarden.java
index 06243eb..9e8c146 100644
--- a/service/java/com/android/server/wifi/ActiveModeWarden.java
+++ b/service/java/com/android/server/wifi/ActiveModeWarden.java
@@ -1373,6 +1373,22 @@
return true;
}
+ private void handleAdditionalClientModeManagerRequest(
+ @NonNull AdditionalClientModeManagerRequestInfo requestInfo) {
+ ClientModeManager cmm = getClientModeManagerInRole(requestInfo.clientRole);
+ if (cmm != null) {
+ // Already have a client mode manager in the requested role.
+ requestInfo.listener.onAnswer(cmm);
+ } else if (shouldRequestAdditionalClientModeManager(requestInfo)) {
+ // Can create an additional client mode manager.
+ startAdditionalClientModeManager(
+ requestInfo.clientRole,
+ requestInfo.listener, requestInfo.requestorWs);
+ } else {
+ requestInfo.listener.onAnswer(getPrimaryClientModeManager());
+ }
+ }
+
@Override
public boolean processMessageFiltered(Message msg) {
@@ -1391,16 +1407,8 @@
}
break;
case CMD_REQUEST_ADDITIONAL_CLIENT_MODE_MANAGER:
- AdditionalClientModeManagerRequestInfo requestInfo =
- (AdditionalClientModeManagerRequestInfo) msg.obj;
- if (shouldRequestAdditionalClientModeManager(requestInfo)) {
- // Can create an additional client mode manager.
- startAdditionalClientModeManager(
- requestInfo.clientRole,
- requestInfo.listener, requestInfo.requestorWs);
- } else {
- requestInfo.listener.onAnswer(getPrimaryClientModeManager());
- }
+ handleAdditionalClientModeManagerRequest(
+ (AdditionalClientModeManagerRequestInfo) msg.obj);
break;
case CMD_REMOVE_ADDITIONAL_CLIENT_MODE_MANAGER:
stopAdditionalClientModeManager((ClientModeManager) msg.obj);
diff --git a/service/java/com/android/server/wifi/WifiNetworkFactory.java b/service/java/com/android/server/wifi/WifiNetworkFactory.java
index 205a9e0..a3d4019 100644
--- a/service/java/com/android/server/wifi/WifiNetworkFactory.java
+++ b/service/java/com/android/server/wifi/WifiNetworkFactory.java
@@ -161,7 +161,6 @@
private boolean mIsPeriodicScanPaused = false;
// We sent a new connection request and are waiting for connection success.
private boolean mPendingConnectionSuccess = false;
- private boolean mAwaitingClientModeManagerRetrieval = false;
/**
* Indicates that we have new data to serialize.
*/
@@ -335,7 +334,6 @@
ActiveModeWarden.ExternalClientModeManagerRequestListener {
@Override
public void onAnswer(@Nullable ClientModeManager modeManager) {
- mAwaitingClientModeManagerRetrieval = false;
if (modeManager != null) {
// Remove the mode manager if the associated request is no longer active.
if (mActiveSpecificNetworkRequest == null
@@ -656,32 +654,6 @@
return true;
}
- private void requestClientModeManagerIfNecessary() {
- // If we already have a ClientModeManager instance retrieved or requested for the
- // previously active request, use that to handle the new request. Creating a new one for
- // overlapping requests would result in unnecessary delay in turning down the old one &
- // creating a new one.
- if (mClientModeManager != null) {
- if (mVerboseLoggingEnabled) Log.v(TAG, "Using cached ClientModeManager instance");
- handleClientModeManagerRetrieval();
- } else if (mAwaitingClientModeManagerRetrieval) {
- // Note: If we have a pending request, then we will wait for the answer to that
- // request to perform the next steps.
- if (mVerboseLoggingEnabled) {
- Log.v(TAG, "Waiting for previously requested ClientModeManager instance");
- }
- } else {
- // No ClientModeManager instance retrieved or requested, request a new instance and
- // then perform the next steps.
- if (mVerboseLoggingEnabled) Log.v(TAG, "Requesting new ClientModeManager instance");
- mAwaitingClientModeManagerRetrieval = true;
- mActiveModeWarden.requestLocalOnlyClientModeManager(
- new ClientModeManagerRequestListener(),
- new WorkSource(mActiveSpecificNetworkRequest.getRequestorUid(),
- mActiveSpecificNetworkRequest.getRequestorPackageName()));
- }
- }
-
/**
* Handle new network connection requests.
*
@@ -930,7 +902,11 @@
mUserSelectedNetwork = networkToConnect;
// Request a new CMM for the connection processing.
- requestClientModeManagerIfNecessary();
+ if (mVerboseLoggingEnabled) Log.v(TAG, "Requesting new ClientModeManager instance");
+ mActiveModeWarden.requestLocalOnlyClientModeManager(
+ new ClientModeManagerRequestListener(),
+ new WorkSource(mActiveSpecificNetworkRequest.getRequestorUid(),
+ mActiveSpecificNetworkRequest.getRequestorPackageName()));
}
private void handleConnectToNetworkUserSelection(WifiConfiguration network) {
diff --git a/service/tests/wifitests/src/com/android/server/wifi/ActiveModeWardenTest.java b/service/tests/wifitests/src/com/android/server/wifi/ActiveModeWardenTest.java
index 4fbe7a5..6241fde 100644
--- a/service/tests/wifitests/src/com/android/server/wifi/ActiveModeWardenTest.java
+++ b/service/tests/wifitests/src/com/android/server/wifi/ActiveModeWardenTest.java
@@ -2492,6 +2492,69 @@
verify(externalRequestListener).onAnswer(null);
}
+ public void requestAdditionalClientModeManagerWhenAlreadyPresent(
+ ActiveModeManager.ClientConnectivityRole role) throws Exception {
+ enterClientModeActiveState();
+
+ ConcreteClientModeManager additionalClientModeManager =
+ mock(ConcreteClientModeManager.class);
+ Mutable<ActiveModeManager.Listener<ConcreteClientModeManager>> additionalClientListener =
+ new Mutable<>();
+ doAnswer((invocation) -> {
+ Object[] args = invocation.getArguments();
+ additionalClientListener.value =
+ (ActiveModeManager.Listener<ConcreteClientModeManager>) args[0];
+ return additionalClientModeManager;
+ }).when(mWifiInjector).makeClientModeManager(
+ any(ActiveModeManager.Listener.class), any(), eq(role),
+ anyBoolean());
+ when(additionalClientModeManager.getRole()).thenReturn(role);
+
+ ActiveModeWarden.ExternalClientModeManagerRequestListener externalRequestListener = mock(
+ ActiveModeWarden.ExternalClientModeManagerRequestListener.class);
+ if (role == ROLE_CLIENT_LOCAL_ONLY) {
+ mActiveModeWarden.requestLocalOnlyClientModeManager(
+ externalRequestListener, TEST_WORKSOURCE);
+ } else if (role == ROLE_CLIENT_SECONDARY_LONG_LIVED) {
+ mActiveModeWarden.requestSecondaryLongLivedClientModeManager(
+ externalRequestListener, TEST_WORKSOURCE);
+ } else if (role == ROLE_CLIENT_SECONDARY_TRANSIENT) {
+ mActiveModeWarden.requestSecondaryTransientClientModeManager(
+ externalRequestListener, TEST_WORKSOURCE);
+ }
+ mLooper.dispatchAll();
+ verify(mWifiInjector).makeClientModeManager(
+ any(), eq(TEST_WORKSOURCE), eq(role), anyBoolean());
+ additionalClientListener.value.onStarted(additionalClientModeManager);
+ mLooper.dispatchAll();
+ // Returns the new client mode manager.
+ ArgumentCaptor<ClientModeManager> requestedClientModeManager =
+ ArgumentCaptor.forClass(ClientModeManager.class);
+ verify(externalRequestListener).onAnswer(requestedClientModeManager.capture());
+ assertEquals(additionalClientModeManager, requestedClientModeManager.getValue());
+
+ // request for one more CMM (returns the existing one).
+ if (role == ROLE_CLIENT_LOCAL_ONLY) {
+ mActiveModeWarden.requestLocalOnlyClientModeManager(
+ externalRequestListener, TEST_WORKSOURCE);
+ } else if (role == ROLE_CLIENT_SECONDARY_LONG_LIVED) {
+ mActiveModeWarden.requestSecondaryLongLivedClientModeManager(
+ externalRequestListener, TEST_WORKSOURCE);
+ } else if (role == ROLE_CLIENT_SECONDARY_TRANSIENT) {
+ mActiveModeWarden.requestSecondaryTransientClientModeManager(
+ externalRequestListener, TEST_WORKSOURCE);
+ }
+ mLooper.dispatchAll();
+
+ // Don't make another client mode manager.
+ verify(mWifiInjector, times(1))
+ .makeClientModeManager(any(), any(), eq(role), anyBoolean());
+ // Returns the existing client mode manager.
+ verify(externalRequestListener, times(2)).onAnswer(requestedClientModeManager.capture());
+ assertEquals(additionalClientModeManager, requestedClientModeManager.getValue());
+
+ }
+
@Test
public void requestRemoveLocalOnlyClientModeManager() throws Exception {
// Ensure that we can create more client ifaces.
@@ -2531,6 +2594,17 @@
}
@Test
+ public void requestLocalOnlyClientModeManagerWhenAlreadyPresent() throws Exception {
+ // Ensure that we can create more client ifaces.
+ when(mWifiNative.isItPossibleToCreateStaIface(any())).thenReturn(true);
+ assertTrue(mActiveModeWarden.canRequestMoreClientModeManagers(any()));
+ when(mResources.getBoolean(R.bool.config_wifiMultiStaLocalOnlyConcurrencyEnabled))
+ .thenReturn(true);
+
+ requestAdditionalClientModeManagerWhenAlreadyPresent(ROLE_CLIENT_LOCAL_ONLY);
+ }
+
+ @Test
public void requestRemoveSecondaryLongLivedClientModeManager() throws Exception {
// Ensure that we can create more client ifaces.
when(mWifiNative.isItPossibleToCreateStaIface(any())).thenReturn(true);
@@ -2558,6 +2632,17 @@
}
@Test
+ public void requestSecondaryLongLivedClientModeManagerWhenAlreadyPresent() throws Exception {
+ // Ensure that we can create more client ifaces.
+ when(mWifiNative.isItPossibleToCreateStaIface(any())).thenReturn(true);
+ assertTrue(mActiveModeWarden.canRequestMoreClientModeManagers(any()));
+ when(mResources.getBoolean(R.bool.config_wifiMultiStaLocalOnlyConcurrencyEnabled))
+ .thenReturn(true);
+
+ requestAdditionalClientModeManagerWhenAlreadyPresent(ROLE_CLIENT_SECONDARY_LONG_LIVED);
+ }
+
+ @Test
public void requestRemoveSecondaryTransientClientModeManager() throws Exception {
// Ensure that we can create more client ifaces.
when(mWifiNative.isItPossibleToCreateStaIface(any())).thenReturn(true);
@@ -2585,6 +2670,17 @@
}
@Test
+ public void requestSecondaryTransientClientModeManagerWhenAlreadyPresent() throws Exception {
+ // Ensure that we can create more client ifaces.
+ when(mWifiNative.isItPossibleToCreateStaIface(any())).thenReturn(true);
+ assertTrue(mActiveModeWarden.canRequestMoreClientModeManagers(any()));
+ when(mResources.getBoolean(R.bool.config_wifiMultiStaLocalOnlyConcurrencyEnabled))
+ .thenReturn(true);
+
+ requestAdditionalClientModeManagerWhenAlreadyPresent(ROLE_CLIENT_SECONDARY_TRANSIENT);
+ }
+
+ @Test
public void airplaneModeToggleOnDisablesWifi() throws Exception {
enterClientModeActiveState();
assertInEnabledState();
diff --git a/service/tests/wifitests/src/com/android/server/wifi/WifiNetworkFactoryTest.java b/service/tests/wifitests/src/com/android/server/wifi/WifiNetworkFactoryTest.java
index 428126f..d32cc46 100644
--- a/service/tests/wifitests/src/com/android/server/wifi/WifiNetworkFactoryTest.java
+++ b/service/tests/wifitests/src/com/android/server/wifi/WifiNetworkFactoryTest.java
@@ -1774,44 +1774,6 @@
* Verify handling of request release after starting connection to the network.
*/
@Test
- public void testHandleNetworkReleaseWithSpecifierWhenAwaitingCmRetrieval() throws Exception {
- doNothing().when(mActiveModeWarden).requestLocalOnlyClientModeManager(any(), any());
-
- when(mClock.getElapsedSinceBootMillis()).thenReturn(0L);
-
- sendNetworkRequestAndSetupForUserSelection();
-
- INetworkRequestUserSelectionCallback networkRequestUserSelectionCallback =
- mNetworkRequestUserSelectionCallback.getValue();
- assertNotNull(networkRequestUserSelectionCallback);
-
- // Now trigger user selection to one of the network.
- mSelectedNetwork = WifiConfigurationTestUtil.createPskNetwork();
- mSelectedNetwork.SSID = "\"" + TEST_SSID_1 + "\"";
- sendUserSelectionSelect(networkRequestUserSelectionCallback, mSelectedNetwork);
- mLooper.dispatchAll();
-
- ArgumentCaptor<ActiveModeWarden.ExternalClientModeManagerRequestListener> cmListenerCaptor =
- ArgumentCaptor.forClass(
- ActiveModeWarden.ExternalClientModeManagerRequestListener.class);
- verify(mActiveModeWarden).requestLocalOnlyClientModeManager(
- cmListenerCaptor.capture(), eq(new WorkSource(TEST_UID_1, TEST_PACKAGE_NAME_1)));
- assertNotNull(cmListenerCaptor.getValue());
-
- // Release the request before the CM instance is delivered.
- mWifiNetworkFactory.releaseNetworkFor(mNetworkRequest);
-
- // Now return the CM instance for the previous request.
- cmListenerCaptor.getValue().onAnswer(mClientModeManager);
-
- // Ensure we removed the CM instance since we no longer have any active request.
- verify(mActiveModeWarden).removeClientModeManager(mClientModeManager);
- }
-
- /**
- * Verify handling of request release after starting connection to the network.
- */
- @Test
public void testHandleNetworkReleaseWithSpecifierAfterConnectionStart() throws Exception {
sendNetworkRequestAndSetupForConnectionStatus();
@@ -1959,8 +1921,8 @@
sendUserSelectionSelect(networkRequestUserSelectionCallback, mSelectedNetwork);
mLooper.dispatchAll();
- // Ensure we don't request a new ClientModeManager.
- verify(mActiveModeWarden, times(1)).requestLocalOnlyClientModeManager(any(), any());
+ // Ensure we request a new ClientModeManager.
+ verify(mActiveModeWarden, times(2)).requestLocalOnlyClientModeManager(any(), any());
// Now return the CM instance for the previous request.
cmListenerCaptor.getValue().onAnswer(mClientModeManager);
@@ -2873,7 +2835,7 @@
sendUserSelectionSelect(networkRequestUserSelectionCallback, mSelectedNetwork);
mLooper.dispatchAll();
- verify(mActiveModeWarden).requestLocalOnlyClientModeManager(any(), any());
+ verify(mActiveModeWarden, atLeastOnce()).requestLocalOnlyClientModeManager(any(), any());
// Cancel the periodic scan timer.
mInOrder.verify(mAlarmManager).cancel(mPeriodicScanListenerArgumentCaptor.getValue());
