Get rid of shortSleep() in ConnectivityServiceTest.
Instead, use IdleHandler to wait for things to become idle.
Bug: 22606153
Change-Id: Ic6ab93ad4d336b40962f9be1096629a44b63ee2f
diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java
index 1f4427f..b888698 100644
--- a/services/core/java/com/android/server/ConnectivityService.java
+++ b/services/core/java/com/android/server/ConnectivityService.java
@@ -359,6 +359,9 @@
*/
private static final int EVENT_REGISTER_NETWORK_LISTENER_WITH_INTENT = 31;
+ /** Handler thread used for both of the handlers below. */
+ @VisibleForTesting
+ protected final HandlerThread mHandlerThread;
/** Handler used for internal events. */
final private InternalHandler mHandler;
/** Handler used for incoming {@link NetworkStateTracker} events. */
@@ -614,6 +617,11 @@
}
private LegacyTypeTracker mLegacyTypeTracker = new LegacyTypeTracker();
+ @VisibleForTesting
+ protected HandlerThread createHandlerThread() {
+ return new HandlerThread("ConnectivityServiceThread");
+ }
+
public ConnectivityService(Context context, INetworkManagementService netManager,
INetworkStatsService statsService, INetworkPolicyManager policyManager) {
if (DBG) log("ConnectivityService starting up");
@@ -627,10 +635,10 @@
mDefaultMobileDataRequest = createInternetRequestForTransport(
NetworkCapabilities.TRANSPORT_CELLULAR);
- HandlerThread handlerThread = new HandlerThread("ConnectivityServiceThread");
- handlerThread.start();
- mHandler = new InternalHandler(handlerThread.getLooper());
- mTrackerHandler = new NetworkStateTrackerHandler(handlerThread.getLooper());
+ mHandlerThread = createHandlerThread();
+ mHandlerThread.start();
+ mHandler = new InternalHandler(mHandlerThread.getLooper());
+ mTrackerHandler = new NetworkStateTrackerHandler(mHandlerThread.getLooper());
// setup our unique device name
if (TextUtils.isEmpty(SystemProperties.get("net.hostname"))) {
diff --git a/services/tests/servicestests/src/com/android/server/ConnectivityServiceTest.java b/services/tests/servicestests/src/com/android/server/ConnectivityServiceTest.java
index 72dfe8b..6281ad2 100644
--- a/services/tests/servicestests/src/com/android/server/ConnectivityServiceTest.java
+++ b/services/tests/servicestests/src/com/android/server/ConnectivityServiceTest.java
@@ -48,8 +48,10 @@
import android.os.ConditionVariable;
import android.os.Handler;
import android.os.HandlerThread;
-import android.os.Looper;
import android.os.INetworkManagementService;
+import android.os.Looper;
+import android.os.MessageQueue;
+import android.os.MessageQueue.IdleHandler;
import android.test.AndroidTestCase;
import android.test.suitebuilder.annotation.LargeTest;
import android.util.Log;
@@ -101,11 +103,87 @@
}
}
+ /**
+ * A subclass of HandlerThread that allows callers to wait for it to become idle. waitForIdle
+ * will return immediately if the handler is already idle.
+ */
+ private class IdleableHandlerThread extends HandlerThread {
+ private IdleHandler mIdleHandler;
+
+ public IdleableHandlerThread(String name) {
+ super(name);
+ }
+
+ public void waitForIdle(int timeoutMs) {
+ final ConditionVariable cv = new ConditionVariable();
+ final MessageQueue queue = getLooper().getQueue();
+
+ synchronized (queue) {
+ if (queue.isIdle()) {
+ return;
+ }
+
+ assertNull("BUG: only one idle handler allowed", mIdleHandler);
+ mIdleHandler = new IdleHandler() {
+ public boolean queueIdle() {
+ cv.open();
+ mIdleHandler = null;
+ return false; // Remove the handler.
+ }
+ };
+ queue.addIdleHandler(mIdleHandler);
+ }
+
+ if (!cv.block(timeoutMs)) {
+ fail("HandlerThread " + getName() +
+ " did not become idle after " + timeoutMs + " ms");
+ queue.removeIdleHandler(mIdleHandler);
+ }
+ }
+ }
+
+ // Tests that IdleableHandlerThread works as expected.
+ public void testIdleableHandlerThread() {
+ final int attempts = 50; // Causes the test to take about 200ms on bullhead-eng.
+
+ // Tests that waitForIdle returns immediately if the service is already idle.
+ for (int i = 0; i < attempts; i++) {
+ mService.waitForIdle();
+ }
+
+ // Bring up a network that we can use to send messages to ConnectivityService.
+ ConditionVariable cv = waitForConnectivityBroadcasts(1);
+ mWiFiNetworkAgent = new MockNetworkAgent(TRANSPORT_WIFI);
+ mWiFiNetworkAgent.connect(false);
+ waitFor(cv);
+ Network n = mWiFiNetworkAgent.getNetwork();
+ assertNotNull(n);
+
+ // Tests that calling waitForIdle waits for messages to be processed.
+ for (int i = 0; i < attempts; i++) {
+ mWiFiNetworkAgent.setSignalStrength(i);
+ mService.waitForIdle();
+ assertEquals(i, mCm.getNetworkCapabilities(n).getSignalStrength());
+ }
+
+ // Ensure that not calling waitForIdle causes a race condition.
+ for (int i = 0; i < attempts; i++) {
+ mWiFiNetworkAgent.setSignalStrength(i);
+ if (i != mCm.getNetworkCapabilities(n).getSignalStrength()) {
+ // We hit a race condition, as expected. Pass the test.
+ return;
+ }
+ }
+
+ // No race? There is a bug in this test.
+ fail("expected race condition at least once in " + attempts + " attempts");
+ }
+
private class MockNetworkAgent {
private final WrappedNetworkMonitor mWrappedNetworkMonitor;
private final NetworkInfo mNetworkInfo;
private final NetworkCapabilities mNetworkCapabilities;
- private final Thread mThread;
+ private final IdleableHandlerThread mHandlerThread;
private final ConditionVariable mDisconnected = new ConditionVariable();
private int mScore;
private NetworkAgent mNetworkAgent;
@@ -126,26 +204,27 @@
default:
throw new UnsupportedOperationException("unimplemented network type");
}
- final ConditionVariable initComplete = new ConditionVariable();
- final ConditionVariable networkMonitorAvailable = mService.getNetworkMonitorCreatedCV();
- mThread = new Thread() {
- public void run() {
- Looper.prepare();
- mNetworkAgent = new NetworkAgent(Looper.myLooper(), mServiceContext,
- "Mock" + typeName, mNetworkInfo, mNetworkCapabilities,
- new LinkProperties(), mScore, new NetworkMisc()) {
- public void unwanted() { mDisconnected.open(); }
- };
- initComplete.open();
- Looper.loop();
- }
+ mHandlerThread = new IdleableHandlerThread("Mock-" + typeName);
+ mHandlerThread.start();
+ mNetworkAgent = new NetworkAgent(mHandlerThread.getLooper(), mServiceContext,
+ "Mock-" + typeName, mNetworkInfo, mNetworkCapabilities,
+ new LinkProperties(), mScore, new NetworkMisc()) {
+ public void unwanted() { mDisconnected.open(); }
};
- mThread.start();
- waitFor(initComplete);
- waitFor(networkMonitorAvailable);
+ // Waits for the NetworkAgent to be registered, which includes the creation of the
+ // NetworkMonitor.
+ mService.waitForIdle();
mWrappedNetworkMonitor = mService.getLastCreatedWrappedNetworkMonitor();
}
+ public void waitForIdle(int timeoutMs) {
+ mHandlerThread.waitForIdle(timeoutMs);
+ }
+
+ public void waitForIdle() {
+ waitForIdle(TIMEOUT_MS);
+ }
+
public void adjustScore(int change) {
mScore += change;
mNetworkAgent.sendNetworkScore(mScore);
@@ -156,6 +235,11 @@
mNetworkAgent.sendNetworkCapabilities(mNetworkCapabilities);
}
+ public void setSignalStrength(int signalStrength) {
+ mNetworkCapabilities.setSignalStrength(signalStrength);
+ mNetworkAgent.sendNetworkCapabilities(mNetworkCapabilities);
+ }
+
public void connectWithoutInternet() {
mNetworkInfo.setDetailedState(DetailedState.CONNECTED, null, null);
mNetworkAgent.sendNetworkInfo(mNetworkInfo);
@@ -309,7 +393,6 @@
}
private class WrappedConnectivityService extends ConnectivityService {
- private final ConditionVariable mNetworkMonitorCreated = new ConditionVariable();
private WrappedNetworkMonitor mLastCreatedNetworkMonitor;
public WrappedConnectivityService(Context context, INetworkManagementService netManager,
@@ -318,6 +401,11 @@
}
@Override
+ protected HandlerThread createHandlerThread() {
+ return new IdleableHandlerThread("WrappedConnectivityService");
+ }
+
+ @Override
protected int getDefaultTcpRwnd() {
// Prevent wrapped ConnectivityService from trying to write to SystemProperties.
return 0;
@@ -350,7 +438,6 @@
final WrappedNetworkMonitor monitor = new WrappedNetworkMonitor(context, handler, nai,
defaultRequest);
mLastCreatedNetworkMonitor = monitor;
- mNetworkMonitorCreated.open();
return monitor;
}
@@ -358,10 +445,14 @@
return mLastCreatedNetworkMonitor;
}
- public ConditionVariable getNetworkMonitorCreatedCV() {
- mNetworkMonitorCreated.close();
- return mNetworkMonitorCreated;
+ public void waitForIdle(int timeoutMs) {
+ ((IdleableHandlerThread) mHandlerThread).waitForIdle(timeoutMs);
}
+
+ public void waitForIdle() {
+ waitForIdle(500);
+ }
+
}
private interface Criteria {
@@ -391,18 +482,6 @@
assertTrue(conditionVariable.block(500));
}
- /**
- * This should only be used to verify that nothing happens, in other words that no unexpected
- * changes occur. It should never be used to wait for a specific positive signal to occur.
- */
- private void shortSleep() {
- // TODO: Instead of sleeping, instead wait for all message loops to idle.
- try {
- Thread.sleep(500);
- } catch (InterruptedException e) {
- }
- }
-
@Override
public void setUp() throws Exception {
super.setUp();
@@ -534,11 +613,11 @@
// Test bringing up unvalidated cellular
mCellNetworkAgent = new MockNetworkAgent(TRANSPORT_CELLULAR);
mCellNetworkAgent.connect(false);
- shortSleep();
+ mService.waitForIdle();
verifyActiveNetwork(TRANSPORT_WIFI);
// Test cellular disconnect.
mCellNetworkAgent.disconnect();
- shortSleep();
+ mService.waitForIdle();
verifyActiveNetwork(TRANSPORT_WIFI);
// Test bringing up validated cellular
mCellNetworkAgent = new MockNetworkAgent(TRANSPORT_CELLULAR);
@@ -809,7 +888,7 @@
// This should not trigger spurious onAvailable() callbacks, b/21762680.
mCellNetworkAgent.adjustScore(-1);
- shortSleep();
+ mService.waitForIdle();
wifiNetworkCallback.assertNoCallback();
cellNetworkCallback.assertNoCallback();
assertEquals(mCellNetworkAgent.getNetwork(), mCm.getActiveNetwork());
@@ -843,7 +922,7 @@
// This should not trigger spurious onAvailable() callbacks, b/21762680.
mCellNetworkAgent.adjustScore(-1);
- shortSleep();
+ mService.waitForIdle();
wifiNetworkCallback.assertNoCallback();
cellNetworkCallback.assertNoCallback();
assertEquals(mCellNetworkAgent.getNetwork(), mCm.getActiveNetwork());