From 188bfd36daf623e3c890114b5bd89df9323a20e6 Mon Sep 17 00:00:00 2001 From: Motomu Utsumi Date: Tue, 30 May 2023 14:38:04 +0900 Subject: [PATCH] Delay frozen app sockets close until the cellular modem wakes up Closing TCP sockets sends RST packets. If the cellular modem is idle, sending RST packets will wake the modem up and consume battery. This CL adds delay_destroy_frozen_sockets_version flag. When this flag and destroy_frozen_sockets_version is enabled, ConnectivityService delays closing socket until the cellular modem wakes up. Pending frozen sockets are closed also when cellular network becomes no longer the default network. This CL also adds flag status and pending uids to the dump. Bug: 284900338 Test: FrameworksNetTests Change-Id: I2562568390dda36d02f72afb3a96f824788964c0 --- .../android/server/ConnectivityService.java | 110 +++++++++- .../server/ConnectivityServiceTest.java | 207 ++++++++++++++++-- 2 files changed, 299 insertions(+), 18 deletions(-) diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index 04d0b93073..60523ddd55 100755 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -934,6 +934,15 @@ public class ConnectivityService extends IConnectivityManager.Stub private final Map mSelfCertifiedCapabilityCache = new HashMap<>(); + // Flag to enable the feature of closing frozen app sockets. + private final boolean mDestroyFrozenSockets; + + // Flag to optimize closing frozen app sockets by waiting for the cellular modem to wake up. + private final boolean mDelayDestroyFrozenSockets; + + // Uids that ConnectivityService is pending to close sockets of. + private final Set mPendingFrozenUids = new ArraySet<>(); + /** * Implements support for the legacy "one network per network type" model. * @@ -1772,8 +1781,11 @@ public class ConnectivityService extends IConnectivityManager.Stub mCdmps = null; } - if (mDeps.isAtLeastU() - && mDeps.isFeatureEnabled(context, KEY_DESTROY_FROZEN_SOCKETS_VERSION)) { + mDestroyFrozenSockets = mDeps.isAtLeastU() + && mDeps.isFeatureEnabled(context, KEY_DESTROY_FROZEN_SOCKETS_VERSION); + mDelayDestroyFrozenSockets = mDeps.isAtLeastU() + && mDeps.isFeatureEnabled(context, DELAY_DESTROY_FROZEN_SOCKETS_VERSION); + if (mDestroyFrozenSockets) { final UidFrozenStateChangedCallback frozenStateChangedCallback = new UidFrozenStateChangedCallback() { @Override @@ -2983,26 +2995,109 @@ public class ConnectivityService extends IConnectivityManager.Stub } } + /** + * Check if the cell network is idle. + * @return true if the cell network state is idle + * false if the cell network state is active or unknown + */ + private boolean isCellNetworkIdle() { + final NetworkAgentInfo defaultNai = getDefaultNetwork(); + if (defaultNai == null + || !defaultNai.networkCapabilities.hasTransport(TRANSPORT_CELLULAR)) { + // mNetworkActivityTracker only tracks the activity of the default network. So if the + // cell network is not the default network, cell network state is unknown. + // TODO(b/279380356): Track cell network state when the cell is not the default network + return false; + } + + return !mNetworkActivityTracker.isDefaultNetworkActive(); + } + private void handleFrozenUids(int[] uids, int[] frozenStates) { final ArraySet ownerUids = new ArraySet<>(); for (int i = 0; i < uids.length; i++) { if (frozenStates[i] == UID_FROZEN_STATE_FROZEN) { ownerUids.add(uids[i]); + } else { + mPendingFrozenUids.remove(uids[i]); } } - if (!ownerUids.isEmpty()) { + if (ownerUids.isEmpty()) { + return; + } + + if (mDelayDestroyFrozenSockets && isCellNetworkIdle()) { + // Delay closing sockets to avoid waking the cell modem up. + // Wi-Fi network state is not considered since waking Wi-Fi modem up is much cheaper + // than waking cell modem up. + mPendingFrozenUids.addAll(ownerUids); + } else { try { mDeps.destroyLiveTcpSocketsByOwnerUids(ownerUids); - } catch (Exception e) { + } catch (SocketException | InterruptedIOException | ErrnoException e) { loge("Exception in socket destroy: " + e); } } } + private void closePendingFrozenSockets() { + ensureRunningOnConnectivityServiceThread(); + + try { + mDeps.destroyLiveTcpSocketsByOwnerUids(mPendingFrozenUids); + } catch (SocketException | InterruptedIOException | ErrnoException e) { + loge("Failed to close pending frozen app sockets: " + e); + } + mPendingFrozenUids.clear(); + } + + private void handleReportNetworkActivity(final NetworkActivityParams params) { + mNetworkActivityTracker.handleReportNetworkActivity(params); + + if (mDelayDestroyFrozenSockets + && params.isActive + && params.label == TRANSPORT_CELLULAR + && !mPendingFrozenUids.isEmpty()) { + closePendingFrozenSockets(); + } + } + + /** + * If the cellular network is no longer the default network, close pending frozen sockets. + * + * @param newNetwork new default network + * @param oldNetwork old default network + */ + private void maybeClosePendingFrozenSockets(NetworkAgentInfo newNetwork, + NetworkAgentInfo oldNetwork) { + final boolean isOldNetworkCellular = oldNetwork != null + && oldNetwork.networkCapabilities.hasTransport(TRANSPORT_CELLULAR); + final boolean isNewNetworkCellular = newNetwork != null + && newNetwork.networkCapabilities.hasTransport(TRANSPORT_CELLULAR); + + if (isOldNetworkCellular + && !isNewNetworkCellular + && !mPendingFrozenUids.isEmpty()) { + closePendingFrozenSockets(); + } + } + + private void dumpCloseFrozenAppSockets(IndentingPrintWriter pw) { + pw.println("CloseFrozenAppSockets:"); + pw.increaseIndent(); + pw.print("mDestroyFrozenSockets="); pw.println(mDestroyFrozenSockets); + pw.print("mDelayDestroyFrozenSockets="); pw.println(mDelayDestroyFrozenSockets); + pw.print("mPendingFrozenUids="); pw.println(mPendingFrozenUids); + pw.decreaseIndent(); + } + @VisibleForTesting static final String KEY_DESTROY_FROZEN_SOCKETS_VERSION = "destroy_frozen_sockets_version"; + @VisibleForTesting + static final String DELAY_DESTROY_FROZEN_SOCKETS_VERSION = + "delay_destroy_frozen_sockets_version"; private void enforceInternetPermission() { mContext.enforceCallingOrSelfPermission( @@ -3604,6 +3699,9 @@ public class ConnectivityService extends IConnectivityManager.Stub pw.println(); dumpAvoidBadWifiSettings(pw); + pw.println(); + dumpCloseFrozenAppSockets(pw); + pw.println(); if (!CollectionUtils.contains(args, SHORT_ARG)) { @@ -4671,6 +4769,7 @@ public class ConnectivityService extends IConnectivityManager.Stub // incorrect) behavior. mNetworkActivityTracker.updateDataActivityTracking( null /* newNetwork */, nai); + maybeClosePendingFrozenSockets(null /* newNetwork */, nai); ensureNetworkTransitionWakelock(nai.toShortString()); } } @@ -5877,7 +5976,7 @@ public class ConnectivityService extends IConnectivityManager.Stub } case EVENT_REPORT_NETWORK_ACTIVITY: final NetworkActivityParams arg = (NetworkActivityParams) msg.obj; - mNetworkActivityTracker.handleReportNetworkActivity(arg); + handleReportNetworkActivity(arg); break; case EVENT_MOBILE_DATA_PREFERRED_UIDS_CHANGED: handleMobileDataPreferredUidsChanged(); @@ -9117,6 +9216,7 @@ public class ConnectivityService extends IConnectivityManager.Stub mLingerMonitor.noteLingerDefaultNetwork(oldDefaultNetwork, newDefaultNetwork); } mNetworkActivityTracker.updateDataActivityTracking(newDefaultNetwork, oldDefaultNetwork); + maybeClosePendingFrozenSockets(newDefaultNetwork, oldDefaultNetwork); mProxyTracker.setDefaultProxy(null != newDefaultNetwork ? newDefaultNetwork.linkProperties.getHttpProxy() : null); resetHttpProxyForNonDefaultNetwork(oldDefaultNetwork); diff --git a/tests/unit/java/com/android/server/ConnectivityServiceTest.java b/tests/unit/java/com/android/server/ConnectivityServiceTest.java index f8e31665f8..e5dec56a29 100755 --- a/tests/unit/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/unit/java/com/android/server/ConnectivityServiceTest.java @@ -154,6 +154,7 @@ import static android.net.resolv.aidl.IDnsResolverUnsolicitedEventListener.VALID import static android.os.Process.INVALID_UID; import static android.system.OsConstants.IPPROTO_TCP; +import static com.android.server.ConnectivityService.DELAY_DESTROY_FROZEN_SOCKETS_VERSION; import static com.android.server.ConnectivityService.KEY_DESTROY_FROZEN_SOCKETS_VERSION; import static com.android.server.ConnectivityService.MAX_NETWORK_REQUESTS_PER_SYSTEM_UID; import static com.android.server.ConnectivityService.PREFERENCE_ORDER_MOBILE_DATA_PREFERERRED; @@ -214,6 +215,7 @@ import static org.mockito.ArgumentMatchers.isNull; import static org.mockito.Matchers.anyInt; import static org.mockito.Mockito.any; import static org.mockito.Mockito.atLeastOnce; +import static org.mockito.Mockito.clearInvocations; import static org.mockito.Mockito.doAnswer; import static org.mockito.Mockito.doNothing; import static org.mockito.Mockito.doReturn; @@ -2130,6 +2132,8 @@ public class ConnectivityServiceTest { return true; case KEY_DESTROY_FROZEN_SOCKETS_VERSION: return true; + case DELAY_DESTROY_FROZEN_SOCKETS_VERSION: + return true; default: return super.isFeatureEnabled(context, name); } @@ -2280,7 +2284,9 @@ public class ConnectivityServiceTest { @Override @SuppressWarnings("DirectInvocationOnMock") public void destroyLiveTcpSocketsByOwnerUids(final Set ownerUids) { // Call mocked destroyLiveTcpSocketsByOwnerUids so that test can verify this method call - mDestroySocketsWrapper.destroyLiveTcpSocketsByOwnerUids(ownerUids); + // Create copy of ownerUids so that tests can verify the correct value even if the + // ConnectivityService update the ownerUids after this method call. + mDestroySocketsWrapper.destroyLiveTcpSocketsByOwnerUids(new ArraySet<>(ownerUids)); } final ArrayTrackRecord>.ReadHead mScheduledEvaluationTimeouts = @@ -11290,6 +11296,9 @@ public class ConnectivityServiceTest { } private void doTestInterfaceClassActivityChanged(final int transportType) throws Exception { + final BaseNetdUnsolicitedEventListener netdUnsolicitedEventListener = + getRegisteredNetdUnsolicitedEventListener(); + final int legacyType = transportToLegacyType(transportType); final LinkProperties lp = new LinkProperties(); lp.setInterfaceName(transportToTestIfaceName(transportType)); @@ -11306,12 +11315,8 @@ public class ConnectivityServiceTest { mCm.addDefaultNetworkActiveListener(listener); - ArgumentCaptor netdCallbackCaptor = - ArgumentCaptor.forClass(BaseNetdUnsolicitedEventListener.class); - verify(mMockNetd).registerUnsolicitedEventListener(netdCallbackCaptor.capture()); - // Interface goes to inactive state - netdCallbackCaptor.getValue().onInterfaceClassActivityChanged(false /* isActive */, + netdUnsolicitedEventListener.onInterfaceClassActivityChanged(false /* isActive */, transportType, TIMESTAMP, NETWORK_ACTIVITY_NO_UID); mServiceContext.expectDataActivityBroadcast(legacyType, false /* isActive */, TIMESTAMP); @@ -11319,7 +11324,7 @@ public class ConnectivityServiceTest { assertFalse(mCm.isDefaultNetworkActive()); // Interface goes to active state - netdCallbackCaptor.getValue().onInterfaceClassActivityChanged(true /* isActive */, + netdUnsolicitedEventListener.onInterfaceClassActivityChanged(true /* isActive */, transportType, TIMESTAMP, TEST_PACKAGE_UID); mServiceContext.expectDataActivityBroadcast(legacyType, true /* isActive */, TIMESTAMP); assertTrue(onNetworkActiveCv.block(TEST_CALLBACK_TIMEOUT_MS)); @@ -18566,6 +18571,27 @@ public class ConnectivityServiceTest { anyInt()); } + // UidFrozenStateChangedCallback is added in U API. + // Returning UidFrozenStateChangedCallback directly makes the test fail on T- devices since + // AndroidJUnit4ClassRunner iterates all declared methods and tries to resolve the return type. + // Solve this by wrapping it in an AtomicReference. Because of erasure, this removes the + // resolving problem as the type isn't seen dynamically. + private AtomicReference getUidFrozenStateChangedCallback() { + ArgumentCaptor activityManagerCallbackCaptor = + ArgumentCaptor.forClass(UidFrozenStateChangedCallback.class); + verify(mActivityManager).registerUidFrozenStateChangedCallback(any(), + activityManagerCallbackCaptor.capture()); + return new AtomicReference<>(activityManagerCallbackCaptor.getValue()); + } + + private BaseNetdUnsolicitedEventListener getRegisteredNetdUnsolicitedEventListener() + throws RemoteException { + ArgumentCaptor netdCallbackCaptor = + ArgumentCaptor.forClass(BaseNetdUnsolicitedEventListener.class); + verify(mMockNetd).registerUnsolicitedEventListener(netdCallbackCaptor.capture()); + return netdCallbackCaptor.getValue(); + } + private static final int TEST_FROZEN_UID = 1000; private static final int TEST_UNFROZEN_UID = 2000; @@ -18576,22 +18602,177 @@ public class ConnectivityServiceTest { @Test @IgnoreUpTo(Build.VERSION_CODES.TIRAMISU) public void testFrozenUidSocketDestroy() throws Exception { - ArgumentCaptor callbackArg = - ArgumentCaptor.forClass(UidFrozenStateChangedCallback.class); - - verify(mActivityManager).registerUidFrozenStateChangedCallback(any(), - callbackArg.capture()); + final UidFrozenStateChangedCallback callback = + getUidFrozenStateChangedCallback().get(); final int[] uids = {TEST_FROZEN_UID, TEST_UNFROZEN_UID}; final int[] frozenStates = {UID_FROZEN_STATE_FROZEN, UID_FROZEN_STATE_UNFROZEN}; - callbackArg.getValue().onUidFrozenStateChanged(uids, frozenStates); + callback.onUidFrozenStateChanged(uids, frozenStates); waitForIdle(); verify(mDestroySocketsWrapper).destroyLiveTcpSocketsByOwnerUids(Set.of(TEST_FROZEN_UID)); } + private void doTestDelayFrozenUidSocketDestroy(int transportType, + boolean freezeWithNetworkInactive, boolean expectDelay) throws Exception { + final TestNetworkCallback defaultCallback = new TestNetworkCallback(); + final LinkProperties lp = new LinkProperties(); + lp.setInterfaceName(transportToTestIfaceName(transportType)); + final TestNetworkAgentWrapper agent = new TestNetworkAgentWrapper(transportType, lp); + testAndCleanup(() -> { + final UidFrozenStateChangedCallback uidFrozenStateChangedCallback = + getUidFrozenStateChangedCallback().get(); + final BaseNetdUnsolicitedEventListener netdUnsolicitedEventListener = + getRegisteredNetdUnsolicitedEventListener(); + + mCm.registerDefaultNetworkCallback(defaultCallback); + agent.connect(true); + defaultCallback.expectAvailableThenValidatedCallbacks(agent); + if (freezeWithNetworkInactive) { + // Make network inactive + netdUnsolicitedEventListener.onInterfaceClassActivityChanged(false /* isActive */, + transportType, TIMESTAMP, NETWORK_ACTIVITY_NO_UID); + } + + // Freeze TEST_FROZEN_UID and TEST_UNFROZEN_UID + final int[] uids1 = {TEST_FROZEN_UID, TEST_UNFROZEN_UID}; + final int[] frozenStates1 = {UID_FROZEN_STATE_FROZEN, UID_FROZEN_STATE_FROZEN}; + uidFrozenStateChangedCallback.onUidFrozenStateChanged(uids1, frozenStates1); + waitForIdle(); + + if (expectDelay) { + verify(mDestroySocketsWrapper, never()).destroyLiveTcpSocketsByOwnerUids(any()); + } else { + verify(mDestroySocketsWrapper).destroyLiveTcpSocketsByOwnerUids( + Set.of(TEST_FROZEN_UID, TEST_UNFROZEN_UID)); + clearInvocations(mDestroySocketsWrapper); + } + + // Unfreeze TEST_UNFROZEN_UID + final int[] uids2 = {TEST_UNFROZEN_UID}; + final int[] frozenStates2 = {UID_FROZEN_STATE_UNFROZEN}; + uidFrozenStateChangedCallback.onUidFrozenStateChanged(uids2, frozenStates2); + + // Make network active + netdUnsolicitedEventListener.onInterfaceClassActivityChanged(true /* isActive */, + transportType, TIMESTAMP, TEST_PACKAGE_UID); + waitForIdle(); + + if (expectDelay) { + verify(mDestroySocketsWrapper).destroyLiveTcpSocketsByOwnerUids( + Set.of(TEST_FROZEN_UID)); + } else { + verify(mDestroySocketsWrapper, never()).destroyLiveTcpSocketsByOwnerUids(any()); + } + }, () -> { // Cleanup + agent.disconnect(); + }, () -> { + mCm.unregisterNetworkCallback(defaultCallback); + }); + } + + @Test + @IgnoreUpTo(Build.VERSION_CODES.TIRAMISU) + public void testDelayFrozenUidSocketDestroy_ActiveCellular() throws Exception { + doTestDelayFrozenUidSocketDestroy(TRANSPORT_CELLULAR, + false /* freezeWithNetworkInactive */, false /* expectDelay */); + } + + @Test + @IgnoreUpTo(Build.VERSION_CODES.TIRAMISU) + public void testDelayFrozenUidSocketDestroy_InactiveCellular() throws Exception { + // When the default network is cellular and cellular network is inactive, closing socket + // is delayed. + doTestDelayFrozenUidSocketDestroy(TRANSPORT_CELLULAR, + true /* freezeWithNetworkInactive */, true /* expectDelay */); + } + + @Test + @IgnoreUpTo(Build.VERSION_CODES.TIRAMISU) + public void testDelayFrozenUidSocketDestroy_ActiveWifi() throws Exception { + doTestDelayFrozenUidSocketDestroy(TRANSPORT_WIFI, + false /* freezeWithNetworkInactive */, false /* expectDelay */); + } + + @Test + @IgnoreUpTo(Build.VERSION_CODES.TIRAMISU) + public void testDelayFrozenUidSocketDestroy_InactiveWifi() throws Exception { + doTestDelayFrozenUidSocketDestroy(TRANSPORT_WIFI, + true /* freezeWithNetworkInactive */, false /* expectDelay */); + } + + /** + * @param switchToWifi if true, simulate a migration of the default network to wifi + * if false, simulate a cell disconnection + */ + private void doTestLoseCellDefaultNetwork_ClosePendingFrozenSockets(final boolean switchToWifi) + throws Exception { + final UidFrozenStateChangedCallback uidFrozenStateChangedCallback = + getUidFrozenStateChangedCallback().get(); + final BaseNetdUnsolicitedEventListener netdUnsolicitedEventListener = + getRegisteredNetdUnsolicitedEventListener(); + + final LinkProperties wifiLp = new LinkProperties(); + wifiLp.setInterfaceName(WIFI_IFNAME); + mWiFiAgent = new TestNetworkAgentWrapper(TRANSPORT_WIFI, wifiLp); + + final LinkProperties cellLp = new LinkProperties(); + cellLp.setInterfaceName(MOBILE_IFNAME); + mCellAgent = new TestNetworkAgentWrapper(TRANSPORT_CELLULAR, cellLp); + + final TestNetworkCallback defaultCallback = new TestNetworkCallback(); + mCm.registerDefaultNetworkCallback(defaultCallback); + try { + mCellAgent.connect(true); + defaultCallback.expectAvailableThenValidatedCallbacks(mCellAgent); + + // Make cell network inactive + netdUnsolicitedEventListener.onInterfaceClassActivityChanged(false /* isActive */, + TRANSPORT_CELLULAR, TIMESTAMP, NETWORK_ACTIVITY_NO_UID); + + // Freeze TEST_FROZEN_UID + final int[] uids = {TEST_FROZEN_UID}; + final int[] frozenStates = {UID_FROZEN_STATE_FROZEN}; + uidFrozenStateChangedCallback.onUidFrozenStateChanged(uids, frozenStates); + waitForIdle(); + + // Closing frozen sockets should be delayed since the default network is cellular + // and cellular network is inactive. + verify(mDestroySocketsWrapper, never()).destroyLiveTcpSocketsByOwnerUids(any()); + + if (switchToWifi) { + mWiFiAgent.connect(true); + defaultCallback.expectAvailableDoubleValidatedCallbacks(mWiFiAgent); + } else { + mCellAgent.disconnect(); + waitForIdle(); + } + + // Pending frozen sockets should be closed since the cellular network is no longer the + // default network. + verify(mDestroySocketsWrapper) + .destroyLiveTcpSocketsByOwnerUids(Set.of(TEST_FROZEN_UID)); + } finally { + mCm.unregisterNetworkCallback(defaultCallback); + } + } + + @Test + @IgnoreUpTo(Build.VERSION_CODES.TIRAMISU) + public void testLoseCellDefaultNetwork_SwitchToWifi_ClosePendingFrozenSockets() + throws Exception { + doTestLoseCellDefaultNetwork_ClosePendingFrozenSockets(true /* switchToWifi */); + } + + @Test + @IgnoreUpTo(Build.VERSION_CODES.TIRAMISU) + public void testLoseCellDefaultNetwork_NoDefaultNetwork_ClosePendingFrozenSockets() + throws Exception { + doTestLoseCellDefaultNetwork_ClosePendingFrozenSockets(false /* switchToWifi */); + } + @Test public void testDisconnectSuspendedNetworkStopClatd() throws Exception { final TestNetworkCallback networkCallback = new TestNetworkCallback();