From a68fb676e66ff1d3e701dac64a6891b9254d50db Mon Sep 17 00:00:00 2001 From: Lorenzo Colitti Date: Wed, 6 Jul 2016 19:04:26 +0900 Subject: [PATCH] Prepare to delete useless loop in handleReleaseNetworkRequest. As explained in the TODO, the loop serves no purpose since only one network can be satisfying a given request at a time. Instead of looping, look up the nai in the mNetworkForRequestId array that exists for this purpose. Keep the loop around with an Slog.wtf statement on it so we can see if we ever hit it, and add a TODO to delete it if we don't. Bug: 23113288 Change-Id: I173de4bd45c5a4169b7a062a981f2ecccaa44143 --- .../android/server/ConnectivityService.java | 44 ++++++++++--------- 1 file changed, 24 insertions(+), 20 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index fb5b3f882e..995a910112 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -2445,33 +2445,37 @@ public class ConnectivityService extends IConnectivityManager.Stub } mNetworkRequestInfoLogs.log("RELEASE " + nri); if (nri.request.isRequest()) { + boolean wasKept = false; + NetworkAgentInfo nai = mNetworkForRequestId.get(nri.request.requestId); + if (nai != null) { + nai.removeRequest(nri.request.requestId); + if (VDBG) { + log(" Removing from current network " + nai.name() + + ", leaving " + nai.numNetworkRequests() + " requests."); + } + if (unneeded(nai)) { + if (DBG) log("no live requests for " + nai.name() + "; disconnecting"); + teardownUnneededNetwork(nai); + } else { + wasKept = true; + } + mNetworkForRequestId.remove(nri.request.requestId); + } + + // TODO: remove this code once we know that the Slog.wtf is never hit. + // // Find all networks that are satisfying this request and remove the request // from their request lists. // TODO - it's my understanding that for a request there is only a single // network satisfying it, so this loop is wasteful - boolean wasKept = false; - for (NetworkAgentInfo nai : mNetworkAgentInfos.values()) { - if (nai.isSatisfyingRequest(nri.request.requestId)) { - nai.removeRequest(nri.request.requestId); - if (VDBG) { - log(" Removing from current network " + nai.name() + - ", leaving " + nai.numNetworkRequests() + " requests."); - } - if (unneeded(nai)) { - if (DBG) log("no live requests for " + nai.name() + "; disconnecting"); - teardownUnneededNetwork(nai); - } else { - // suspect there should only be one pass through here - // but if any were kept do the check below - wasKept |= true; - } + for (NetworkAgentInfo otherNai : mNetworkAgentInfos.values()) { + if (otherNai.isSatisfyingRequest(nri.request.requestId) && otherNai != nai) { + Slog.wtf(TAG, "Request " + nri.request + " satisfied by " + + otherNai.name() + ", but mNetworkAgentInfos says " + + (nai != null ? nai.name() : "null")); } } - NetworkAgentInfo nai = mNetworkForRequestId.get(nri.request.requestId); - if (nai != null) { - mNetworkForRequestId.remove(nri.request.requestId); - } // Maintain the illusion. When this request arrived, we might have pretended // that a network connected to serve it, even though the network was already // connected. Now that this request has gone away, we might have to pretend