From 4fce5d1bc7678af59e5cf83bcb64845780f47750 Mon Sep 17 00:00:00 2001 From: James Mattis Date: Thu, 12 Nov 2020 15:53:42 -0800 Subject: [PATCH 1/3] Allow a way to track the active request in an NRI As we are now allowing for multi-layered requests in NetworkRequestInfo (NRI), we need a way to track which of those requests ended up being satisfied. This also includes updates to NetworkRequestInfo itself to support multilayared requests. Bug: 173146509 Bug: 171991028 Test: atest FrameworksNetTests atest FrameworksNetIntegrationTests atest CtsNetTestCasesLatestSdk Change-Id: I7bb5a564769c90928871fe28de05195c9cfae6b5 Change-Id: Ibf37f94b53eb2e833821553e00d76fe38bfea266 --- .../android/server/ConnectivityService.java | 37 ++++++++++++++++--- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index fba1e79ad7..ab11ff86e0 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -5350,6 +5350,12 @@ public class ConnectivityService extends IConnectivityManager.Stub } } + private void ensureAllNetworkRequestsHaveType(List requests) { + for (int i = 0; i < requests.size(); i++) { + ensureNetworkRequestHasType(requests.get(i)); + } + } + private void ensureNetworkRequestHasType(NetworkRequest request) { if (request.type == NetworkRequest.Type.NONE) { throw new IllegalArgumentException( @@ -5380,7 +5386,7 @@ public class ConnectivityService extends IConnectivityManager.Stub NetworkRequestInfo(NetworkRequest r, PendingIntent pi) { request = r; mRequests = initializeRequests(r); - ensureNetworkRequestHasType(request); + ensureAllNetworkRequestsHaveType(mRequests); mPendingIntent = pi; messenger = null; mBinder = null; @@ -5394,7 +5400,7 @@ public class ConnectivityService extends IConnectivityManager.Stub messenger = m; request = r; mRequests = initializeRequests(r); - ensureNetworkRequestHasType(request); + ensureAllNetworkRequestsHaveType(mRequests); mBinder = binder; mPid = getCallingPid(); mUid = getCallingUid(); @@ -5418,6 +5424,19 @@ public class ConnectivityService extends IConnectivityManager.Stub return Collections.unmodifiableList(tempRequests); } + private NetworkRequest getSatisfiedRequest() { + if (mSatisfier == null) { + return null; + } + + for (NetworkRequest req : mRequests) { + if (mSatisfier.isSatisfyingRequest(req.requestId)) { + return req; + } + } + + return null; + } private void enforceRequestCountLimit() { synchronized (mUidToNetworkRequestCount) { @@ -5436,14 +5455,16 @@ public class ConnectivityService extends IConnectivityManager.Stub } } + @Override public void binderDied() { log("ConnectivityService NetworkRequestInfo binderDied(" + - request + ", " + mBinder + ")"); - releaseNetworkRequest(request); + mRequests + ", " + mBinder + ")"); + releaseNetworkRequest(mRequests); } + @Override public String toString() { - return "uid/pid:" + mUid + "/" + mPid + " " + request + return "uid/pid:" + mUid + "/" + mPid + " " + mRequests + (mPendingIntent == null ? "" : " to trigger " + mPendingIntent); } } @@ -5763,6 +5784,12 @@ public class ConnectivityService extends IConnectivityManager.Stub return mNextNetworkProviderId.getAndIncrement(); } + private void releaseNetworkRequest(List networkRequests) { + for (int i = 0; i < networkRequests.size(); i++) { + releaseNetworkRequest(networkRequests.get(i)); + } + } + @Override public void releaseNetworkRequest(NetworkRequest networkRequest) { ensureNetworkRequestHasType(networkRequest); From 258ea3c3c33b21b0a3e23170e3edf0a1e49c3005 Mon Sep 17 00:00:00 2001 From: James Mattis Date: Sun, 15 Nov 2020 15:04:40 -0800 Subject: [PATCH 2/3] Update requestsSortedById() to sort by collection Update requestsSortedById() to sort NetworkRequestInfo by their nested collection of NetworkRequest objects vs a single request. Before the NetworkRequestInfo with the request with the lowest requestId would be sorted to the top. Now the NetworkRequestInfo which contains the request with the lowest requestId will be sorted to the top. Bug: 173292541 Bug: 171991028 Test: atest FrameworksNetTests Change-Id: I51e3c00d59443e37ddbf168c423d13df8d14fa64 --- .../android/server/ConnectivityService.java | 13 +++++-- .../server/ConnectivityServiceTest.java | 38 +++++++++++++++++++ 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index ab11ff86e0..4480479979 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -2697,10 +2697,16 @@ public class ConnectivityService extends IConnectivityManager.Stub /** * Return an array of all current NetworkRequest sorted by request id. */ - private NetworkRequestInfo[] requestsSortedById() { + @VisibleForTesting + protected NetworkRequestInfo[] requestsSortedById() { NetworkRequestInfo[] requests = new NetworkRequestInfo[0]; requests = mNetworkRequests.values().toArray(requests); - Arrays.sort(requests, Comparator.comparingInt(nri -> nri.request.requestId)); + // Sort the array based off the NRI containing the min requestId in its requests. + Arrays.sort(requests, + Comparator.comparingInt(nri -> Collections.min(nri.mRequests, + Comparator.comparingInt(req -> req.requestId)).requestId + ) + ); return requests; } @@ -5367,7 +5373,8 @@ public class ConnectivityService extends IConnectivityManager.Stub * Tracks info about the requester. * Also used to notice when the calling process dies so we can self-expire */ - private class NetworkRequestInfo implements IBinder.DeathRecipient { + @VisibleForTesting + protected class NetworkRequestInfo implements IBinder.DeathRecipient { final List mRequests; final NetworkRequest request; diff --git a/tests/net/java/com/android/server/ConnectivityServiceTest.java b/tests/net/java/com/android/server/ConnectivityServiceTest.java index 6293befb2c..408dd0a835 100644 --- a/tests/net/java/com/android/server/ConnectivityServiceTest.java +++ b/tests/net/java/com/android/server/ConnectivityServiceTest.java @@ -259,7 +259,10 @@ import org.mockito.MockitoAnnotations; import org.mockito.Spy; import org.mockito.stubbing.Answer; +import java.io.FileDescriptor; import java.io.IOException; +import java.io.PrintWriter; +import java.io.StringWriter; import java.net.DatagramSocket; import java.net.Inet4Address; import java.net.Inet6Address; @@ -7585,4 +7588,39 @@ public class ConnectivityServiceTest { mCm.unregisterNetworkCallback(networkCallback); } + + @Test + public void testDumpDoesNotCrash() { + StringWriter stringWriter = new StringWriter(); + + mService.dump(new FileDescriptor(), new PrintWriter(stringWriter), new String[0]); + + assertFalse(stringWriter.toString().isEmpty()); + } + + @Test + public void testRequestsSortedByIdSortsCorrectly() { + final TestNetworkCallback genericNetworkCallback = new TestNetworkCallback(); + final TestNetworkCallback wifiNetworkCallback = new TestNetworkCallback(); + final TestNetworkCallback cellNetworkCallback = new TestNetworkCallback(); + final NetworkRequest genericRequest = new NetworkRequest.Builder() + .clearCapabilities().build(); + final NetworkRequest wifiRequest = new NetworkRequest.Builder() + .addTransportType(TRANSPORT_WIFI).build(); + final NetworkRequest cellRequest = new NetworkRequest.Builder() + .addTransportType(TRANSPORT_CELLULAR).build(); + mCm.registerNetworkCallback(genericRequest, genericNetworkCallback); + mCm.registerNetworkCallback(wifiRequest, wifiNetworkCallback); + mCm.registerNetworkCallback(cellRequest, cellNetworkCallback); + + ConnectivityService.NetworkRequestInfo[] nriOutput = mService.requestsSortedById(); + + assertTrue(nriOutput.length > 1); + for (int i = 0; i < nriOutput.length - 1; i++) { + boolean isRequestIdInOrder = + nriOutput[i].mRequests.get(0).requestId + < nriOutput[i + 1].mRequests.get(0).requestId; + assertTrue(isRequestIdInOrder); + } + } } From e955141cc61032a2a3c77870ebeb457449708194 Mon Sep 17 00:00:00 2001 From: James Mattis Date: Sun, 15 Nov 2020 20:22:01 -0800 Subject: [PATCH 3/3] Update toString() to use the correct request Update ConnectivityService.NetworkReassignment#toString to use either the current satisfier's request otherwise highest priority request when executing toString(). This is part of the mulilayered request changes. Bug: 173336774 Bug: 171991028 Test: atest FrameworksNetTests Change-Id: Ibed6cdd4522133164b2b919f62ecc9411943f026 --- services/core/java/com/android/server/ConnectivityService.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 4480479979..6c95f22355 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -6814,7 +6814,7 @@ public class ConnectivityService extends IConnectivityManager.Stub } public String toString() { - return mRequest.request.requestId + " : " + return mRequest.mRequests.get(0).requestId + " : " + (null != mOldNetwork ? mOldNetwork.network.netId : "null") + " → " + (null != mNewNetwork ? mNewNetwork.network.netId : "null"); }