From 74128521ad2c1d8709493a9cafd44b25480839ae Mon Sep 17 00:00:00 2001 From: paulhu Date: Tue, 28 Sep 2021 02:29:03 +0000 Subject: [PATCH] Stop using PerUidCounter#transact PerUidCounter#transact is used to adjust the request counter for the per-app API flows. Directly adjusting the counter is not ideal however in the per-app flows, the nris can't be removed until they are used to create the new nris upon set. In fact, satisfiers are the info that new nris need reference. Without satisfiers in new nris, the avaiable callbacks would be sent to listeners agin when assign new satisfiers. Even the new best networks are same as previous satisfiers, but the new nris have lost those info if calling handleRemoveNetworkRequests() before createPerAppCallbackRequestsToRegister(). However, removing satisfiers from nris is not necessary actually because the CS will update the best network to nri when compute network reassignment. It doesn't need to be cleared when calling handleRemoveNetworkRequest(). Thus, keep that info and adjust the sequence to remove nri first. The counter is still correct and doesn't hit limit artificially. Bug: 201648050 Test: atest FrameworksNetTests CtsNetTestCases Change-Id: I4cbc953def7866b23c2b8ebc8deaadf0ffc3b75d --- .../android/server/ConnectivityService.java | 82 ++++--------------- 1 file changed, 14 insertions(+), 68 deletions(-) diff --git a/service/src/com/android/server/ConnectivityService.java b/service/src/com/android/server/ConnectivityService.java index c69af0601e..33b08c2628 100644 --- a/service/src/com/android/server/ConnectivityService.java +++ b/service/src/com/android/server/ConnectivityService.java @@ -1200,35 +1200,6 @@ public class ConnectivityService extends IConnectivityManager.Stub mUidToNetworkRequestCount.put(uid, newRequestCount); } } - - /** - * Used to adjust the request counter for the per-app API flows. Directly adjusting the - * counter is not ideal however in the per-app flows, the nris can't be removed until they - * are used to create the new nris upon set. Therefore the request count limit can be - * artificially hit. This method is used as a workaround for this particular case so that - * the request counts are accounted for correctly. - * @param uid the uid to adjust counts for - * @param numOfNewRequests the new request count to account for - * @param r the runnable to execute - */ - public void transact(final int uid, final int numOfNewRequests, @NonNull final Runnable r) { - // This should only be used on the handler thread as per all current and foreseen - // use-cases. ensureRunningOnConnectivityServiceThread() can't be used because there is - // no ref to the outer ConnectivityService. - synchronized (mUidToNetworkRequestCount) { - final int reqCountOverage = getCallingUidRequestCountOverage(uid, numOfNewRequests); - decrementCount(uid, reqCountOverage); - r.run(); - incrementCountOrThrow(uid, reqCountOverage); - } - } - - private int getCallingUidRequestCountOverage(final int uid, final int numOfNewRequests) { - final int newUidRequestCount = mUidToNetworkRequestCount.get(uid, 0) - + numOfNewRequests; - return newUidRequestCount >= MAX_NETWORK_REQUESTS_PER_SYSTEM_UID - ? newUidRequestCount - (MAX_NETWORK_REQUESTS_PER_SYSTEM_UID - 1) : 0; - } } /** @@ -4285,8 +4256,6 @@ public class ConnectivityService extends IConnectivityManager.Stub if (null != nri.getActiveRequest()) { if (!nri.getActiveRequest().isListen()) { removeSatisfiedNetworkRequestFromNetwork(nri); - } else { - nri.setSatisfier(null, null); } } @@ -4351,7 +4320,6 @@ public class ConnectivityService extends IConnectivityManager.Stub } else { wasKept = true; } - nri.setSatisfier(null, null); if (!wasBackgroundNetwork && nai.isBackgroundNetwork()) { // Went from foreground to background. updateCapabilitiesForNetwork(nai); @@ -10090,13 +10058,9 @@ public class ConnectivityService extends IConnectivityManager.Stub validateNetworkCapabilitiesOfProfileNetworkPreference(preference.capabilities); mProfileNetworkPreferences = mProfileNetworkPreferences.plus(preference); - mSystemNetworkRequestCounter.transact( - mDeps.getCallingUid(), mProfileNetworkPreferences.preferences.size(), - () -> { - final ArraySet nris = - createNrisFromProfileNetworkPreferences(mProfileNetworkPreferences); - replaceDefaultNetworkRequestsForPreference(nris, PREFERENCE_ORDER_PROFILE); - }); + removeDefaultNetworkRequestsForPreference(PREFERENCE_ORDER_PROFILE); + addPerAppDefaultNetworkRequests( + createNrisFromProfileNetworkPreferences(mProfileNetworkPreferences)); // Finally, rematch. rematchAllNetworksAndRequests(); @@ -10141,14 +10105,9 @@ public class ConnectivityService extends IConnectivityManager.Stub private void handleMobileDataPreferredUidsChanged() { mMobileDataPreferredUids = ConnectivitySettingsManager.getMobileDataPreferredUids(mContext); - mSystemNetworkRequestCounter.transact( - mDeps.getCallingUid(), 1 /* numOfNewRequests */, - () -> { - final ArraySet nris = - createNrisFromMobileDataPreferredUids(mMobileDataPreferredUids); - replaceDefaultNetworkRequestsForPreference(nris, - PREFERENCE_ORDER_MOBILE_DATA_PREFERERRED); - }); + removeDefaultNetworkRequestsForPreference(PREFERENCE_ORDER_MOBILE_DATA_PREFERERRED); + addPerAppDefaultNetworkRequests( + createNrisFromMobileDataPreferredUids(mMobileDataPreferredUids)); // Finally, rematch. rematchAllNetworksAndRequests(); } @@ -10237,16 +10196,9 @@ public class ConnectivityService extends IConnectivityManager.Stub } mOemNetworkPreferencesLogs.log("UPDATE INITIATED: " + preference); - final int uniquePreferenceCount = new ArraySet<>( - preference.getNetworkPreferences().values()).size(); - mSystemNetworkRequestCounter.transact( - mDeps.getCallingUid(), uniquePreferenceCount, - () -> { - final ArraySet nris = - new OemNetworkRequestFactory() - .createNrisFromOemNetworkPreferences(preference); - replaceDefaultNetworkRequestsForPreference(nris, PREFERENCE_ORDER_OEM); - }); + removeDefaultNetworkRequestsForPreference(PREFERENCE_ORDER_OEM); + addPerAppDefaultNetworkRequests(new OemNetworkRequestFactory() + .createNrisFromOemNetworkPreferences(preference)); mOemNetworkPreferences = preference; if (null != listener) { @@ -10258,14 +10210,12 @@ public class ConnectivityService extends IConnectivityManager.Stub } } - private void replaceDefaultNetworkRequestsForPreference( - @NonNull final Set nris, final int preferenceOrder) { + private void removeDefaultNetworkRequestsForPreference(final int preferenceOrder) { // Skip the requests which are set by other network preference. Because the uid range rules // should stay in netd. final Set requests = new ArraySet<>(mDefaultNetworkRequests); requests.removeIf(request -> request.mPreferenceOrder != preferenceOrder); handleRemoveNetworkRequests(requests); - addPerAppDefaultNetworkRequests(nris); } private void addPerAppDefaultNetworkRequests(@NonNull final Set nris) { @@ -10274,14 +10224,10 @@ public class ConnectivityService extends IConnectivityManager.Stub final ArraySet perAppCallbackRequestsToUpdate = getPerAppCallbackRequestsToUpdate(); final ArraySet nrisToRegister = new ArraySet<>(nris); - mSystemNetworkRequestCounter.transact( - mDeps.getCallingUid(), perAppCallbackRequestsToUpdate.size(), - () -> { - nrisToRegister.addAll( - createPerAppCallbackRequestsToRegister(perAppCallbackRequestsToUpdate)); - handleRemoveNetworkRequests(perAppCallbackRequestsToUpdate); - handleRegisterNetworkRequests(nrisToRegister); - }); + handleRemoveNetworkRequests(perAppCallbackRequestsToUpdate); + nrisToRegister.addAll( + createPerAppCallbackRequestsToRegister(perAppCallbackRequestsToUpdate)); + handleRegisterNetworkRequests(nrisToRegister); } /**