[NS A03] Remove mNetworkForRequestId
The goal of doing this is to remove the locks. Locking imposes
an ordering that is parallel to the program sequence ; often
it is what is needed, but the refactoring in progress needs to
change the traversal order of the requests without having
observable side-effects (or at least, very controlled ones).
The order is difficult enough to maintain when there is
only one. In this case, it is easy to remove the locks, and it
will simplify vastly the complexity of the refactoring by
removing the parallel ordering that would have to be maintained.
Bug: 113554781
Test: ConnectivityServiceTest
Change-Id: Ie4ae6a97053559e6cbac8230f2db3d35000b35a9
diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java
index 944899b..1a86239 100644
--- a/services/core/java/com/android/server/ConnectivityService.java
+++ b/services/core/java/com/android/server/ConnectivityService.java
@@ -3061,7 +3061,8 @@
// A network factory has connected. Send it all current NetworkRequests.
for (NetworkRequestInfo nri : mNetworkRequests.values()) {
if (nri.request.isListen()) continue;
- NetworkAgentInfo nai = getNetworkForRequest(nri);
+ ensureRunningOnConnectivityServiceThread();
+ NetworkAgentInfo nai = nri.mSatisfier;
final int score;
final int serial;
if (nai != null) {
@@ -3165,14 +3166,16 @@
for (int i = 0; i < nai.numNetworkRequests(); i++) {
NetworkRequest request = nai.requestAt(i);
final NetworkRequestInfo nri = mNetworkRequests.get(request);
- final NetworkAgentInfo currentNetwork = getNetworkForRequest(nri);
+ ensureRunningOnConnectivityServiceThread();
+ final NetworkAgentInfo currentNetwork = nri.mSatisfier;
if (currentNetwork != null && currentNetwork.network.netId == nai.network.netId) {
- setNetworkForRequest(nri, null);
+ nri.mSatisfier = null;
sendUpdatedScoreToFactories(request, null);
}
}
nai.clearLingerState();
if (nai.isSatisfyingRequest(mDefaultRequest.requestId)) {
+ mDefaultNetworkNai = null;
updateDataActivityTracking(null /* newNetwork */, nai);
notifyLockdownVpn(nai);
ensureNetworkTransitionWakelock(nai.name());
@@ -3268,7 +3271,8 @@
}
}
rematchAllNetworksAndRequests(null, 0);
- if (nri.request.isRequest() && getNetworkForRequest(nri) == null) {
+ ensureRunningOnConnectivityServiceThread();
+ if (nri.request.isRequest() && nri.mSatisfier == null) {
sendUpdatedScoreToFactories(nri.request, null);
}
}
@@ -3315,6 +3319,7 @@
// If this Network is already the highest scoring Network for a request, or if
// there is hope for it to become one if it validated, then it is needed.
+ ensureRunningOnConnectivityServiceThread();
if (nri.request.isRequest() && nai.satisfies(nri.request) &&
(nai.isSatisfyingRequest(nri.request.requestId) ||
// Note that this catches two important cases:
@@ -3324,7 +3329,7 @@
// 2. Unvalidated WiFi will not be reaped when validated cellular
// is currently satisfying the request. This is desirable when
// WiFi ends up validating and out scoring cellular.
- getNetworkForRequest(nri).getCurrentScore()
+ nri.mSatisfier.getCurrentScore()
< nai.getCurrentScoreAsValidated())) {
return false;
}
@@ -3353,7 +3358,8 @@
if (mNetworkRequests.get(nri.request) == null) {
return;
}
- if (getNetworkForRequest(nri) != null) {
+ ensureRunningOnConnectivityServiceThread();
+ if (nri.mSatisfier != null) {
return;
}
if (VDBG || (DBG && nri.request.isRequest())) {
@@ -3401,7 +3407,8 @@
mNetworkRequestInfoLogs.log("RELEASE " + nri);
if (nri.request.isRequest()) {
boolean wasKept = false;
- final NetworkAgentInfo nai = getNetworkForRequest(nri);
+ ensureRunningOnConnectivityServiceThread();
+ final NetworkAgentInfo nai = nri.mSatisfier;
if (nai != null) {
boolean wasBackgroundNetwork = nai.isBackgroundNetwork();
nai.removeRequest(nri.request.requestId);
@@ -3418,7 +3425,7 @@
} else {
wasKept = true;
}
- setNetworkForRequest(nri, null);
+ nri.mSatisfier = null;
if (!wasBackgroundNetwork && nai.isBackgroundNetwork()) {
// Went from foreground to background.
updateCapabilities(nai.getCurrentScore(), nai, nai.networkCapabilities);
@@ -5492,16 +5499,6 @@
if (DBG) log("unregisterNetworkFactory for " + nfi.name);
}
- /**
- * NetworkAgentInfo supporting a request by requestId.
- * These have already been vetted (their Capabilities satisfy the request)
- * and the are the highest scored network available.
- * the are keyed off the Requests requestId.
- */
- // NOTE: Accessed on multiple threads, must be synchronized on itself.
- @GuardedBy("mNetworkForRequestId")
- private final SparseArray<NetworkAgentInfo> mNetworkForRequestId = new SparseArray<>();
-
// NOTE: Accessed on multiple threads, must be synchronized on itself.
@GuardedBy("mNetworkForNetId")
private final SparseArray<NetworkAgentInfo> mNetworkForNetId = new SparseArray<>();
@@ -5533,23 +5530,6 @@
// priority networks like ethernet are active.
private final NetworkRequest mDefaultWifiRequest;
- @Nullable
- private NetworkAgentInfo getNetworkForRequest(@NonNull final NetworkRequestInfo requestInfo) {
- ensureRunningOnConnectivityServiceThread();
- synchronized (mNetworkForRequestId) {
- return requestInfo.mSatisfier;
- }
- }
-
- private void setNetworkForRequest(@NonNull final NetworkRequestInfo requestInfo,
- @Nullable NetworkAgentInfo nai) {
- ensureRunningOnConnectivityServiceThread();
- synchronized (mNetworkForRequestId) {
- requestInfo.mSatisfier = nai;
- if (isDefaultRequest(requestInfo)) mDefaultNetworkNai = nai;
- }
- }
-
private NetworkAgentInfo getDefaultNetwork() {
return mDefaultNetworkNai;
}
@@ -6269,7 +6249,7 @@
}
}
- private void makeDefault(NetworkAgentInfo newNetwork) {
+ private void makeDefault(@NonNull final NetworkAgentInfo newNetwork) {
if (DBG) log("Switching to new default network: " + newNetwork);
try {
@@ -6278,6 +6258,7 @@
loge("Exception setting default network :" + e);
}
+ mDefaultNetworkNai = newNetwork;
notifyLockdownVpn(newNetwork);
handleApplyDefaultProxy(newNetwork.linkProperties.getHttpProxy());
updateTcpBufferSizes(newNetwork.linkProperties.getTcpBufferSizes());
@@ -6365,7 +6346,8 @@
// requests or not, and doesn't affect the network's score.
if (nri.request.isListen()) continue;
- final NetworkAgentInfo currentNetwork = getNetworkForRequest(nri);
+ ensureRunningOnConnectivityServiceThread();
+ final NetworkAgentInfo currentNetwork = nri.mSatisfier;
final boolean satisfies = newNetwork.satisfies(nri.request);
if (newNetwork == currentNetwork && satisfies) {
if (VDBG) {
@@ -6399,7 +6381,7 @@
if (VDBG || DDBG) log(" accepting network in place of null");
}
newNetwork.unlingerRequest(nri.request);
- setNetworkForRequest(nri, newNetwork);
+ nri.mSatisfier = newNetwork;
if (!newNetwork.addRequest(nri.request)) {
Slog.wtf(TAG, "BUG: " + newNetwork.name() + " already has " + nri.request);
}
@@ -6433,12 +6415,13 @@
}
newNetwork.removeRequest(nri.request.requestId);
if (currentNetwork == newNetwork) {
- setNetworkForRequest(nri, null);
+ nri.mSatisfier = null;
+ if (isDefaultRequest(nri)) mDefaultNetworkNai = null;
sendUpdatedScoreToFactories(nri.request, null);
} else {
Slog.wtf(TAG, "BUG: Removing request " + nri.request.requestId + " from " +
newNetwork.name() +
- " without updating mNetworkForRequestId or factories!");
+ " without updating mSatisfier or factories!");
}
// TODO: Technically, sending CALLBACK_LOST here is
// incorrect if there is a replacement network currently