ConnectivityServiceTest: remove flaky waitForIdle test.

This patch removes testNotWaitingForIdleCausesRaceConditions() from
ConnectivityServiceTest because it is in nature flaky and prevents using
ConnectivityServiceTest as a patch presubmit check. Estimated failure
rate is 1/15 on Nexus 6p.

This patch also simplifies how ConnectivityServiceTest waits for
handlers to become idle by removing IdleableHandlerThread and turning it
into a signle static method.

Test: $ runtest frameworks-net
Bug: 31479480
Change-Id: I2d78709cbb61d5d01cd59cff326469417f73f1ab

services/core/java/com/android/server/ConnectivityService.java

@@ -690,11 +690,6 @@ public class ConnectivityService extends IConnectivityManager.Stub
     }
     private LegacyTypeTracker mLegacyTypeTracker = new LegacyTypeTracker();
 
-    @VisibleForTesting
-    protected HandlerThread createHandlerThread() {
-        return new HandlerThread("ConnectivityServiceThread");
-    }
-
     public ConnectivityService(Context context, INetworkManagementService netManager,
             INetworkStatsService statsService, INetworkPolicyManager policyManager) {
         this(context, netManager, statsService, policyManager, new IpConnectivityLog());
@@ -715,7 +710,7 @@ public class ConnectivityService extends IConnectivityManager.Stub
         mDefaultMobileDataRequest = createInternetRequestForTransport(
                 NetworkCapabilities.TRANSPORT_CELLULAR, NetworkRequest.Type.BACKGROUND_REQUEST);
 
-        mHandlerThread = createHandlerThread();
+        mHandlerThread = new HandlerThread("ConnectivityServiceThread");
         mHandlerThread.start();
         mHandler = new InternalHandler(mHandlerThread.getLooper());
         mTrackerHandler = new NetworkStateTrackerHandler(mHandlerThread.getLooper());

tests/net/java/com/android/server/ConnectivityServiceTest.java

@@ -68,7 +68,6 @@ import android.os.Process;
 import android.os.SystemClock;
 import android.provider.Settings;
 import android.test.AndroidTestCase;
-import android.test.FlakyTest;
 import android.test.mock.MockContentResolver;
 import android.test.suitebuilder.annotation.SmallTest;
 import android.util.Log;
@@ -154,49 +153,32 @@ public class ConnectivityServiceTest extends AndroidTestCase {
     }
 
     /**
-     * A subclass of HandlerThread that allows callers to wait for it to become idle. waitForIdle
+     * Block until the given handler becomes idle, or until timeoutMs has passed.
-     * will return immediately if the handler is already idle.
      */
-    private class IdleableHandlerThread extends HandlerThread {
+    private static void waitForIdleHandler(HandlerThread handler, int timeoutMs) {
-        private IdleHandler mIdleHandler;
-
-        public IdleableHandlerThread(String name) {
-            super(name);
-        }
-
-        public void waitForIdle(int timeoutMs) {
         final ConditionVariable cv = new ConditionVariable();
-            final MessageQueue queue = getLooper().getQueue();
+        final MessageQueue queue = handler.getLooper().getQueue();
-
+        final IdleHandler idleHandler = () -> {
+            synchronized (queue) {
+                cv.open();
+                return false; // Remove the idleHandler.
+            }
+        };
         synchronized (queue) {
             if (queue.isIdle()) {
                 return;
             }
-
+            queue.addIdleHandler(idleHandler);
-                assertNull("BUG: only one idle handler allowed", mIdleHandler);
-                mIdleHandler = new IdleHandler() {
-                    public boolean queueIdle() {
-                        synchronized (queue) {
-                            cv.open();
-                            mIdleHandler = null;
-                            return false;  // Remove the handler.
         }
-                    }
-                };
-                queue.addIdleHandler(mIdleHandler);
-            }
-
         if (!cv.block(timeoutMs)) {
-                fail("HandlerThread " + getName() +
+            fail("HandlerThread " + handler.getName() +
                     " did not become idle after " + timeoutMs + " ms");
-                queue.removeIdleHandler(mIdleHandler);
+            queue.removeIdleHandler(idleHandler);
-            }
         }
     }
 
-    // Tests that IdleableHandlerThread works as expected.
     @SmallTest
-    public void testIdleableHandlerThread() {
+    public void testWaitForIdle() {
         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.
@@ -220,9 +202,9 @@ public class ConnectivityServiceTest extends AndroidTestCase {
         }
     }
 
-    @SmallTest
+    // This test has an inherent race condition in it, and cannot be enabled for continuous testing
-    @FlakyTest(tolerance = 3)
+    // or presubmit tests. It is kept for manual runs and documentation purposes.
-    public void testNotWaitingForIdleCausesRaceConditions() {
+    public void verifyThatNotWaitingForIdleCausesRaceConditions() {
         // Bring up a network that we can use to send messages to ConnectivityService.
         ConditionVariable cv = waitForConnectivityBroadcasts(1);
         mWiFiNetworkAgent = new MockNetworkAgent(TRANSPORT_WIFI);
@@ -249,7 +231,7 @@ public class ConnectivityServiceTest extends AndroidTestCase {
         private final WrappedNetworkMonitor mWrappedNetworkMonitor;
         private final NetworkInfo mNetworkInfo;
         private final NetworkCapabilities mNetworkCapabilities;
-        private final IdleableHandlerThread mHandlerThread;
+        private final HandlerThread mHandlerThread;
         private final ConditionVariable mDisconnected = new ConditionVariable();
         private final ConditionVariable mNetworkStatusReceived = new ConditionVariable();
         private final ConditionVariable mPreventReconnectReceived = new ConditionVariable();
@@ -281,7 +263,7 @@ public class ConnectivityServiceTest extends AndroidTestCase {
                 default:
                     throw new UnsupportedOperationException("unimplemented network type");
             }
-            mHandlerThread = new IdleableHandlerThread("Mock-" + typeName);
+            mHandlerThread = new HandlerThread("Mock-" + typeName);
             mHandlerThread.start();
             mNetworkAgent = new NetworkAgent(mHandlerThread.getLooper(), mServiceContext,
                     "Mock-" + typeName, mNetworkInfo, mNetworkCapabilities,
@@ -321,7 +303,7 @@ public class ConnectivityServiceTest extends AndroidTestCase {
         }
 
         public void waitForIdle(int timeoutMs) {
-            mHandlerThread.waitForIdle(timeoutMs);
+            waitForIdleHandler(mHandlerThread, timeoutMs);
         }
 
         public void waitForIdle() {
@@ -647,11 +629,6 @@ public class ConnectivityServiceTest extends AndroidTestCase {
             mLingerDelayMs = TEST_LINGER_DELAY_MS;
         }
 
-        @Override
-        protected HandlerThread createHandlerThread() {
-            return new IdleableHandlerThread("WrappedConnectivityService");
-        }
-
         @Override
         protected int getDefaultTcpRwnd() {
             // Prevent wrapped ConnectivityService from trying to write to SystemProperties.
@@ -710,7 +687,7 @@ public class ConnectivityServiceTest extends AndroidTestCase {
         }
 
         public void waitForIdle(int timeoutMs) {
-            ((IdleableHandlerThread) mHandlerThread).waitForIdle(timeoutMs);
+            waitForIdleHandler(mHandlerThread, timeoutMs);
         }
 
         public void waitForIdle() {