From 0b7a74842b89975a678f4e406ad3d0f512dfa702 Mon Sep 17 00:00:00 2001 From: Erik Kline Date: Wed, 25 Nov 2015 12:49:38 +0900 Subject: [PATCH] Support timeouts for requestNetwork() invocations. [DO NOT MERGE] Bug: 21414325 Change-Id: I08118be8e8cf92fc406d431e99a6c9191a863ff3 --- .../java/android/net/ConnectivityManager.java | 27 ++- .../android/server/ConnectivityService.java | 181 ++++++++++-------- .../server/ConnectivityServiceTest.java | 87 ++++++++- 3 files changed, 210 insertions(+), 85 deletions(-) diff --git a/core/java/android/net/ConnectivityManager.java b/core/java/android/net/ConnectivityManager.java index 8e91d2eeb0..2b99fa12b7 100644 --- a/core/java/android/net/ConnectivityManager.java +++ b/core/java/android/net/ConnectivityManager.java @@ -2221,7 +2221,8 @@ public class ConnectivityManager { /** * Called if no network is found in the given timeout time. If no timeout is given, - * this will not be called. + * this will not be called. The associated {@link NetworkRequest} will have already + * been removed and released, as if {@link #unregisterNetworkCallback} had been called. * @hide */ public void onUnavailable() {} @@ -2294,6 +2295,26 @@ public class ConnectivityManager { /** @hide */ public static final int CALLBACK_RESUMED = BASE + 12; + /** @hide */ + public static String getCallbackName(int whichCallback) { + switch (whichCallback) { + case CALLBACK_PRECHECK: return "CALLBACK_PRECHECK"; + case CALLBACK_AVAILABLE: return "CALLBACK_AVAILABLE"; + case CALLBACK_LOSING: return "CALLBACK_LOSING"; + case CALLBACK_LOST: return "CALLBACK_LOST"; + case CALLBACK_UNAVAIL: return "CALLBACK_UNAVAIL"; + case CALLBACK_CAP_CHANGED: return "CALLBACK_CAP_CHANGED"; + case CALLBACK_IP_CHANGED: return "CALLBACK_IP_CHANGED"; + case CALLBACK_RELEASED: return "CALLBACK_RELEASED"; + case CALLBACK_EXIT: return "CALLBACK_EXIT"; + case EXPIRE_LEGACY_REQUEST: return "EXPIRE_LEGACY_REQUEST"; + case CALLBACK_SUSPENDED: return "CALLBACK_SUSPENDED"; + case CALLBACK_RESUMED: return "CALLBACK_RESUMED"; + default: + return Integer.toString(whichCallback); + } + } + private class CallbackHandler extends Handler { private final HashMapmCallbackMap; private final AtomicInteger mRefCount; @@ -2458,7 +2479,7 @@ public class ConnectivityManager { private final static int REQUEST = 2; private NetworkRequest sendRequestForNetwork(NetworkCapabilities need, - NetworkCallback networkCallback, int timeoutSec, int action, + NetworkCallback networkCallback, int timeoutMs, int action, int legacyType) { if (networkCallback == null) { throw new IllegalArgumentException("null NetworkCallback"); @@ -2472,7 +2493,7 @@ public class ConnectivityManager { new Messenger(sCallbackHandler), new Binder()); } else { networkCallback.networkRequest = mService.requestNetwork(need, - new Messenger(sCallbackHandler), timeoutSec, new Binder(), legacyType); + new Messenger(sCallbackHandler), timeoutMs, new Binder(), legacyType); } if (networkCallback.networkRequest != null) { sNetworkCallback.put(networkCallback.networkRequest, networkCallback); diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index c80a82e7dc..63d91e811a 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -2321,90 +2321,104 @@ public class ConnectivityService extends IConnectivityManager.Stub return true; } - private void handleReleaseNetworkRequest(NetworkRequest request, int callingUid) { - NetworkRequestInfo nri = mNetworkRequests.get(request); - if (nri != null) { - if (Process.SYSTEM_UID != callingUid && nri.mUid != callingUid) { - if (DBG) log("Attempt to release unowned NetworkRequest " + request); - return; - } - if (DBG) log("releasing NetworkRequest " + request); - nri.unlinkDeathRecipient(); - mNetworkRequests.remove(request); - mNetworkRequestInfoLogs.log("RELEASE " + nri); - if (nri.isRequest) { - // 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.networkRequests.get(nri.request.requestId) != null) { - nai.networkRequests.remove(nri.request.requestId); - if (DBG) { - log(" Removing from current network " + nai.name() + - ", leaving " + nai.networkRequests.size() + - " 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; - } - } - } - - 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 - // that the network disconnected. LegacyTypeTracker will generate that - // phantom disconnect for this type. - if (nri.request.legacyType != TYPE_NONE && nai != null) { - boolean doRemove = true; - if (wasKept) { - // check if any of the remaining requests for this network are for the - // same legacy type - if so, don't remove the nai - for (int i = 0; i < nai.networkRequests.size(); i++) { - NetworkRequest otherRequest = nai.networkRequests.valueAt(i); - if (otherRequest.legacyType == nri.request.legacyType && - isRequest(otherRequest)) { - if (DBG) log(" still have other legacy request - leaving"); - doRemove = false; - } - } - } - - if (doRemove) { - mLegacyTypeTracker.remove(nri.request.legacyType, nai, false); - } - } - - for (NetworkFactoryInfo nfi : mNetworkFactoryInfos.values()) { - nfi.asyncChannel.sendMessage(android.net.NetworkFactory.CMD_CANCEL_REQUEST, - nri.request); - } - } else { - // listens don't have a singular affectedNetwork. Check all networks to see - // if this listen request applies and remove it. - for (NetworkAgentInfo nai : mNetworkAgentInfos.values()) { - nai.networkRequests.remove(nri.request.requestId); - if (nri.request.networkCapabilities.hasSignalStrength() && - nai.satisfiesImmutableCapabilitiesOf(nri.request)) { - updateSignalStrengthThresholds(nai, "RELEASE", nri.request); - } - } - } - callCallbackForRequest(nri, null, ConnectivityManager.CALLBACK_RELEASED); + private void handleTimedOutNetworkRequest(final NetworkRequestInfo nri) { + if (mNetworkRequests.get(nri.request) != null && mNetworkForRequestId.get( + nri.request.requestId) == null) { + handleRemoveNetworkRequest(nri, Process.SYSTEM_UID, + ConnectivityManager.CALLBACK_UNAVAIL); } } + private void handleReleaseNetworkRequest(NetworkRequest request, int callingUid) { + final NetworkRequestInfo nri = mNetworkRequests.get(request); + if (nri != null) { + handleRemoveNetworkRequest(nri, callingUid, ConnectivityManager.CALLBACK_RELEASED); + } + } + + private void handleRemoveNetworkRequest( + final NetworkRequestInfo nri, final int callingUid, final int whichCallback) { + if (Process.SYSTEM_UID != callingUid && nri.mUid != callingUid) { + if (DBG) log("Attempt to release unowned NetworkRequest " + nri.request); + return; + } + final String logCallbackType = ConnectivityManager.getCallbackName(whichCallback); + if (DBG) log("releasing NetworkRequest " + nri.request + " (" + logCallbackType + ")"); + nri.unlinkDeathRecipient(); + mNetworkRequests.remove(nri.request); + mNetworkRequestInfoLogs.log("RELEASE " + nri + " (" + logCallbackType + ")"); + if (nri.isRequest) { + // 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.networkRequests.get(nri.request.requestId) != null) { + nai.networkRequests.remove(nri.request.requestId); + if (DBG) { + log(" Removing from current network " + nai.name() + + ", leaving " + nai.networkRequests.size() + + " 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; + } + } + } + + 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 + // that the network disconnected. LegacyTypeTracker will generate that + // phantom disconnect for this type. + if (nri.request.legacyType != TYPE_NONE && nai != null) { + boolean doRemove = true; + if (wasKept) { + // check if any of the remaining requests for this network are for the + // same legacy type - if so, don't remove the nai + for (int i = 0; i < nai.networkRequests.size(); i++) { + NetworkRequest otherRequest = nai.networkRequests.valueAt(i); + if (otherRequest.legacyType == nri.request.legacyType && + isRequest(otherRequest)) { + if (DBG) log(" still have other legacy request - leaving"); + doRemove = false; + } + } + } + + if (doRemove) { + mLegacyTypeTracker.remove(nri.request.legacyType, nai, false); + } + } + + for (NetworkFactoryInfo nfi : mNetworkFactoryInfos.values()) { + nfi.asyncChannel.sendMessage(android.net.NetworkFactory.CMD_CANCEL_REQUEST, + nri.request); + } + } else { + // listens don't have a singular affectedNetwork. Check all networks to see + // if this listen request applies and remove it. + for (NetworkAgentInfo nai : mNetworkAgentInfos.values()) { + nai.networkRequests.remove(nri.request.requestId); + if (nri.request.networkCapabilities.hasSignalStrength() && + nai.satisfiesImmutableCapabilitiesOf(nri.request)) { + updateSignalStrengthThresholds(nai, "RELEASE", nri.request); + } + } + } + callCallbackForRequest(nri, null, whichCallback); + } + public void setAcceptUnvalidated(Network network, boolean accept, boolean always) { enforceConnectivityInternalPermission(); mHandler.sendMessage(mHandler.obtainMessage(EVENT_SET_ACCEPT_UNVALIDATED, @@ -2552,6 +2566,11 @@ public class ConnectivityService extends IConnectivityManager.Stub handleRegisterNetworkRequestWithIntent(msg); break; } + case EVENT_TIMEOUT_NETWORK_REQUEST: { + NetworkRequestInfo nri = (NetworkRequestInfo) msg.obj; + handleTimedOutNetworkRequest(nri); + break; + } case EVENT_RELEASE_NETWORK_REQUEST_WITH_INTENT: { handleReleaseNetworkRequestWithIntent((PendingIntent) msg.obj, msg.arg1); break; diff --git a/services/tests/servicestests/src/com/android/server/ConnectivityServiceTest.java b/services/tests/servicestests/src/com/android/server/ConnectivityServiceTest.java index 7fe8158864..85a83e6ee6 100644 --- a/services/tests/servicestests/src/com/android/server/ConnectivityServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/ConnectivityServiceTest.java @@ -960,7 +960,8 @@ public class ConnectivityServiceTest extends AndroidTestCase { NONE, AVAILABLE, LOSING, - LOST + LOST, + UNAVAILABLE } /** @@ -990,6 +991,13 @@ public class ConnectivityServiceTest extends AndroidTestCase { mConditionVariable.open(); } + @Override + public void onUnavailable() { + assertEquals(CallbackState.NONE, mLastCallback); + mLastCallback = CallbackState.UNAVAILABLE; + mConditionVariable.open(); + } + void expectCallback(CallbackState state) { waitFor(mConditionVariable); assertEquals(state, mLastCallback); @@ -1352,6 +1360,83 @@ public class ConnectivityServiceTest extends AndroidTestCase { execptionCalled); } + /** + * Validate that a satisfied network request does not trigger onUnavailable() once the + * time-out period expires. + */ + @SmallTest + public void testSatisfiedNetworkRequestDoesNotTriggerOnUnavailable() { + NetworkRequest nr = new NetworkRequest.Builder().addTransportType( + NetworkCapabilities.TRANSPORT_WIFI).build(); + final TestNetworkCallback networkCallback = new TestNetworkCallback(); + mCm.requestNetwork(nr, networkCallback, 10); + + mWiFiNetworkAgent = new MockNetworkAgent(TRANSPORT_WIFI); + mWiFiNetworkAgent.connect(false); + networkCallback.expectCallback(CallbackState.AVAILABLE); + + // pass timeout and validate that UNAVAILABLE is not called + try { + Thread.sleep(15); + } catch (InterruptedException e) { + } + networkCallback.assertNoCallback(); + } + + /** + * Validate that when a time-out is specified for a network request the onUnavailable() + * callback is called when time-out expires. Then validate that if network request is + * (somehow) satisfied - the callback isn't called later. + */ + @SmallTest + public void testTimedoutNetworkRequest() { + NetworkRequest nr = new NetworkRequest.Builder().addTransportType( + NetworkCapabilities.TRANSPORT_WIFI).build(); + final TestNetworkCallback networkCallback = new TestNetworkCallback(); + mCm.requestNetwork(nr, networkCallback, 10); + + // pass timeout and validate that UNAVAILABLE is called + try { + Thread.sleep(15); + } catch (InterruptedException e) { + } + networkCallback.expectCallback(CallbackState.UNAVAILABLE); + + // create a network satisfying request - validate that request not triggered + mWiFiNetworkAgent = new MockNetworkAgent(TRANSPORT_WIFI); + mWiFiNetworkAgent.connect(false); + networkCallback.assertNoCallback(); + } + + /** + * Validate that when a network request is unregistered (cancelled) the time-out for that + * request doesn't trigger the onUnavailable() callback. + */ + @SmallTest + public void testTimedoutAfterUnregisteredNetworkRequest() { + NetworkRequest nr = new NetworkRequest.Builder().addTransportType( + NetworkCapabilities.TRANSPORT_WIFI).build(); + final TestNetworkCallback networkCallback = new TestNetworkCallback(); + mCm.requestNetwork(nr, networkCallback, 10); + + // remove request + mCm.unregisterNetworkCallback(networkCallback); + + // pass timeout and validate that no callbacks + // Note: doesn't validate that nothing called from CS since even if called the CM already + // unregisters the callback and won't pass it through! + try { + Thread.sleep(15); + } catch (InterruptedException e) { + } + networkCallback.assertNoCallback(); + + // create a network satisfying request - validate that request not triggered + mWiFiNetworkAgent = new MockNetworkAgent(TRANSPORT_WIFI); + mWiFiNetworkAgent.connect(false); + networkCallback.assertNoCallback(); + } + private static class TestKeepaliveCallback extends PacketKeepaliveCallback { public static enum CallbackType { ON_STARTED, ON_STOPPED, ON_ERROR };