From d99da81faed29c0533561783432ba4a20d012fc9 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. (cherry-pick of 0b7a74842b89975a678f4e406ad3d0f512dfa702) (cherry picked from commit 155a59aa6323ba22e75b86ee84c1409468e06a69) Bug: 21414325 Change-Id: I1a58823a372154589f972b98c4c428eab0e0523e --- .../java/android/net/ConnectivityManager.java | 110 +++++++++++------- .../android/server/ConnectivityService.java | 27 ++++- .../server/ConnectivityServiceTest.java | 81 ++++++++++++- 3 files changed, 169 insertions(+), 49 deletions(-) diff --git a/core/java/android/net/ConnectivityManager.java b/core/java/android/net/ConnectivityManager.java index 51431ebfaf..43c8c81da2 100644 --- a/core/java/android/net/ConnectivityManager.java +++ b/core/java/android/net/ConnectivityManager.java @@ -2079,8 +2079,6 @@ public class ConnectivityManager { @SystemApi public void startTethering(int type, boolean showProvisioningUi, final OnStartTetheringCallback callback, Handler handler) { - checkNotNull(callback, "OnStartTetheringCallback cannot be null."); - ResultReceiver wrappedCallback = new ResultReceiver(handler) { @Override protected void onReceiveResult(int resultCode, Bundle resultData) { @@ -2091,7 +2089,6 @@ public class ConnectivityManager { } } }; - try { mService.startTethering(type, wrappedCallback, showProvisioningUi); } catch (RemoteException e) { @@ -2660,7 +2657,6 @@ public class ConnectivityManager { public static final int CALLBACK_IP_CHANGED = BASE + 7; /** @hide */ public static final int CALLBACK_RELEASED = BASE + 8; - // TODO: consider deleting CALLBACK_EXIT and shifting following enum codes down by 1. /** @hide */ public static final int CALLBACK_EXIT = BASE + 9; /** @hide obj = NetworkCapabilities, arg1 = seq number */ @@ -2691,17 +2687,24 @@ public class ConnectivityManager { } private class CallbackHandler extends Handler { + private final HashMapmCallbackMap; + private final AtomicInteger mRefCount; private static final String TAG = "ConnectivityManager.CallbackHandler"; + private final ConnectivityManager mCm; private static final boolean DBG = false; - CallbackHandler(Looper looper) { + CallbackHandler(Looper looper, HashMapcallbackMap, + AtomicInteger refCount, ConnectivityManager cm) { super(looper); + mCallbackMap = callbackMap; + mRefCount = refCount; + mCm = cm; } @Override public void handleMessage(Message message) { - NetworkRequest request = getObject(message, NetworkRequest.class); - Network network = getObject(message, Network.class); + NetworkRequest request = (NetworkRequest) getObject(message, NetworkRequest.class); + Network network = (Network) getObject(message, Network.class); if (DBG) { Log.d(TAG, whatToString(message.what) + " for network " + network); } @@ -2744,7 +2747,9 @@ public class ConnectivityManager { case CALLBACK_CAP_CHANGED: { NetworkCallback callback = getCallback(request, "CAP_CHANGED"); if (callback != null) { - NetworkCapabilities cap = getObject(message, NetworkCapabilities.class); + NetworkCapabilities cap = (NetworkCapabilities)getObject(message, + NetworkCapabilities.class); + callback.onCapabilitiesChanged(network, cap); } break; @@ -2752,7 +2757,9 @@ public class ConnectivityManager { case CALLBACK_IP_CHANGED: { NetworkCallback callback = getCallback(request, "IP_CHANGED"); if (callback != null) { - LinkProperties lp = getObject(message, LinkProperties.class); + LinkProperties lp = (LinkProperties)getObject(message, + LinkProperties.class); + callback.onLinkPropertiesChanged(network, lp); } break; @@ -2772,16 +2779,24 @@ public class ConnectivityManager { break; } case CALLBACK_RELEASED: { - final NetworkCallback callback; - synchronized(sCallbacks) { - callback = sCallbacks.remove(request); + NetworkCallback callback = null; + synchronized(mCallbackMap) { + callback = mCallbackMap.remove(request); } - if (callback == null) { + if (callback != null) { + synchronized(mRefCount) { + if (mRefCount.decrementAndGet() == 0) { + getLooper().quit(); + } + } + } else { Log.e(TAG, "callback not found for RELEASED message"); } break; } case CALLBACK_EXIT: { + Log.d(TAG, "Listener quitting"); + getLooper().quit(); break; } case EXPIRE_LEGACY_REQUEST: { @@ -2791,14 +2806,14 @@ public class ConnectivityManager { } } - private T getObject(Message msg, Class c) { - return (T) msg.getData().getParcelable(c.getSimpleName()); + private Object getObject(Message msg, Class c) { + return msg.getData().getParcelable(c.getSimpleName()); } private NetworkCallback getCallback(NetworkRequest req, String name) { NetworkCallback callback; - synchronized(sCallbacks) { - callback = sCallbacks.get(req); + synchronized(mCallbackMap) { + callback = mCallbackMap.get(req); } if (callback == null) { Log.e(TAG, "callback not found for " + name + " message"); @@ -2807,56 +2822,63 @@ public class ConnectivityManager { } } - private CallbackHandler getHandler() { - synchronized (sCallbacks) { - if (sCallbackHandler == null) { - sCallbackHandler = new CallbackHandler(ConnectivityThread.getInstanceLooper()); + private void incCallbackHandlerRefCount() { + synchronized(sCallbackRefCount) { + if (sCallbackRefCount.incrementAndGet() == 1) { + // TODO: switch this to ConnectivityThread + HandlerThread callbackThread = new HandlerThread("ConnectivityManager"); + callbackThread.start(); + sCallbackHandler = new CallbackHandler(callbackThread.getLooper(), + sNetworkCallback, sCallbackRefCount, this); } - return sCallbackHandler; } } - static final HashMap sCallbacks = new HashMap<>(); - static CallbackHandler sCallbackHandler; + private void decCallbackHandlerRefCount() { + synchronized(sCallbackRefCount) { + if (sCallbackRefCount.decrementAndGet() == 0) { + sCallbackHandler.obtainMessage(CALLBACK_EXIT).sendToTarget(); + sCallbackHandler = null; + } + } + } + + static final HashMap sNetworkCallback = + new HashMap(); + static final AtomicInteger sCallbackRefCount = new AtomicInteger(0); + static CallbackHandler sCallbackHandler = null; private final static int LISTEN = 1; private final static int REQUEST = 2; private NetworkRequest sendRequestForNetwork(NetworkCapabilities need, - NetworkCallback callback, int timeoutMs, int action, int legacyType) { - return sendRequestForNetwork(need, callback, getHandler(), timeoutMs, action, legacyType); - } - - private NetworkRequest sendRequestForNetwork(NetworkCapabilities need, - NetworkCallback callback, Handler handler, int timeoutMs, int action, int legacyType) { - if (callback == null) { + NetworkCallback networkCallback, int timeoutMs, int action, + int legacyType) { + if (networkCallback == null) { throw new IllegalArgumentException("null NetworkCallback"); } if (need == null && action != REQUEST) { throw new IllegalArgumentException("null NetworkCapabilities"); } - // TODO: throw an exception if callback.networkRequest is not null. - // http://b/20701525 - final NetworkRequest request; try { - synchronized(sCallbacks) { - Messenger messenger = new Messenger(handler); - Binder binder = new Binder(); + incCallbackHandlerRefCount(); + synchronized(sNetworkCallback) { if (action == LISTEN) { - request = mService.listenForNetwork(need, messenger, binder); + networkCallback.networkRequest = mService.listenForNetwork(need, + new Messenger(sCallbackHandler), new Binder()); } else { - request = mService.requestNetwork( - need, messenger, timeoutMs, binder, legacyType); + networkCallback.networkRequest = mService.requestNetwork(need, + new Messenger(sCallbackHandler), timeoutMs, new Binder(), legacyType); } - if (request != null) { - sCallbacks.put(request, callback); + if (networkCallback.networkRequest != null) { + sNetworkCallback.put(networkCallback.networkRequest, networkCallback); } - callback.networkRequest = request; } } catch (RemoteException e) { throw e.rethrowFromSystemServer(); } - return request; + if (networkCallback.networkRequest == null) decCallbackHandlerRefCount(); + return networkCallback.networkRequest; } /** diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java index 4cd7d41ca9..64ca629bf0 100644 --- a/services/core/java/com/android/server/ConnectivityService.java +++ b/services/core/java/com/android/server/ConnectivityService.java @@ -2591,14 +2591,28 @@ public class ConnectivityService extends IConnectivityManager.Stub "request NetworkCapabilities", ConnectivityManager.CALLBACK_CAP_CHANGED); } + private void handleTimedOutNetworkRequest(final NetworkRequestInfo nri) { + if (mNetworkRequests.get(nri.request) != null && mNetworkForRequestId.get( + nri.request.requestId) == null) { + handleRemoveNetworkRequest(nri, ConnectivityManager.CALLBACK_UNAVAIL); + } + } + private void handleReleaseNetworkRequest(NetworkRequest request, int callingUid) { final NetworkRequestInfo nri = getNriForAppRequest( request, callingUid, "release NetworkRequest"); - if (nri == null) return; + if (nri != null) { + handleRemoveNetworkRequest(nri, ConnectivityManager.CALLBACK_RELEASED); + } + } - if (VDBG || (DBG && nri.request.isRequest())) log("releasing " + request); + private void handleRemoveNetworkRequest(final NetworkRequestInfo nri, final int whichCallback) { + final String logCallbackType = ConnectivityManager.getCallbackName(whichCallback); + if (VDBG || (DBG && nri.request.isRequest())) { + log("releasing " + nri.request + " (" + logCallbackType + ")"); + } nri.unlinkDeathRecipient(); - mNetworkRequests.remove(request); + mNetworkRequests.remove(nri.request); synchronized (mUidToNetworkRequestCount) { int requests = mUidToNetworkRequestCount.get(nri.mUid, 0); if (requests < 1) { @@ -2692,7 +2706,7 @@ public class ConnectivityService extends IConnectivityManager.Stub } } } - callCallbackForRequest(nri, null, ConnectivityManager.CALLBACK_RELEASED, 0); + callCallbackForRequest(nri, null, whichCallback, 0); } @Override @@ -2929,6 +2943,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 cb9d9629db..c5cca251c8 100644 --- a/services/tests/servicestests/src/com/android/server/ConnectivityServiceTest.java +++ b/services/tests/servicestests/src/com/android/server/ConnectivityServiceTest.java @@ -1077,7 +1077,8 @@ public class ConnectivityServiceTest extends AndroidTestCase { NETWORK_CAPABILITIES, LINK_PROPERTIES, LOSING, - LOST + LOST, + UNAVAILABLE } private static class CallbackInfo { @@ -1125,6 +1126,11 @@ public class ConnectivityServiceTest extends AndroidTestCase { setLastCallback(CallbackState.AVAILABLE, network, null); } + @Override + public void onUnavailable() { + setLastCallback(CallbackState.UNAVAILABLE, null, null); + } + @Override public void onLosing(Network network, int maxMsToLive) { setLastCallback(CallbackState.LOSING, network, maxMsToLive /* autoboxed int */); @@ -2236,6 +2242,79 @@ public class ConnectivityServiceTest extends AndroidTestCase { mCm.unregisterNetworkCallback(defaultCallback); } + /** + * 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, mWiFiNetworkAgent); + + // 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 + networkCallback.expectCallback(CallbackState.UNAVAILABLE, null); + + // 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 };