Handle potential race condition when getting busy resources in UiControllerImpl.

espresso/CHANGELOG.md

@@ -19,6 +19,7 @@ The following artifacts were released:
 * Replace now-unnecessary reflection from TestLooperManagerCompat when using Android SDK 36 APIs
 * Don't suppress AppNotIdleException if dumpThreadStates throws.
 * Remove Espresso.onIdle tracing
+* Fix NullPointerException in UiControllerImpl.
 
 **New Features**
 

espresso/core/java/androidx/test/espresso/base/UiControllerImpl.java

@@ -35,7 +35,6 @@
 import androidx.test.espresso.IdlingPolicies;
 import androidx.test.espresso.IdlingPolicy;
 import androidx.test.espresso.InjectEventSecurityException;
-import androidx.test.espresso.UiController;
 import androidx.test.espresso.base.IdlingResourceRegistry.IdleNotificationCallback;
 import androidx.test.espresso.util.StringJoinerKt;
 import androidx.test.espresso.util.concurrent.ThreadFactoryBuilder;
@@ -77,7 +76,8 @@ enum IdleCondition {
     COMPAT_TASKS_HAVE_IDLED,
     KEY_INJECT_HAS_COMPLETED,
     MOTION_INJECTION_HAS_COMPLETED,
-    DYNAMIC_TASKS_HAVE_IDLED;
+    DYNAMIC_TASKS_HAVE_IDLED,
+    POSSIBLE_RACE_CONDITION_DETECTED;
 
     /** Checks whether this condition has been signaled. */
     public boolean isSignaled(BitSet conditionSet) {
@@ -509,8 +509,9 @@ private IdleNotifier<IdleNotificationCallback> loopUntil(
           start + masterIdlePolicy.getIdleTimeoutUnit().toMillis(masterIdlePolicy.getIdleTimeout());
       interrogation = new MainThreadInterrogation(conditions, conditionSet, end);
 
+      Interrogator interrogator = new Interrogator();
       InterrogationStatus result =
-          new Interrogator().loopAndInterrogate(testLooperManager, interrogation);
+          interrogator.loopAndInterrogate(testLooperManager, interrogation);
       if (InterrogationStatus.COMPLETED == result) {
         // did not time out, all conditions happy.
         return dynamicIdle;
@@ -551,12 +552,51 @@ private IdleNotifier<IdleNotificationCallback> loopUntil(
               }
 
               List<String> busyResources = idlingResourceRegistry.getBusyResources();
+
+              if (busyResources == null) {
+                Log.w(
+                    TAG, "Possible race condition detected, looping until race buster is complete");
+                // Null return value indicates a possible race condition.
+                // Flush the queue to allow resolution to take place.
+                controllerHandler.sendMessage(
+                    IdleCondition.POSSIBLE_RACE_CONDITION_DETECTED.createSignal(
+                        controllerHandler, generation));
+
+                // New interrogation to reset timeout and wait only for our signal.
+                // Since we are just emptying the queue this should not ever timeout.
+                start = SystemClock.uptimeMillis();
+                end =
+                    start
+                        + masterIdlePolicy
+                            .getIdleTimeoutUnit()
+                            .toMillis(masterIdlePolicy.getIdleTimeout());
+                interrogation =
+                    new MainThreadInterrogation(
+                        EnumSet.of(IdleCondition.POSSIBLE_RACE_CONDITION_DETECTED),
+                        conditionSet,
+                        end);
+                InterrogationStatus raceBusterResult =
+                    interrogator.loopAndInterrogate(testLooperManager, interrogation);
+                if (raceBusterResult == InterrogationStatus.COMPLETED) {
+                  // Can still return null if resource is buggy.
+                  busyResources = idlingResourceRegistry.getBusyResources();
+                } else {
+                  Log.w(
+                      TAG,
+                      String.format(
+                          Locale.ROOT,
+                          "Failed to resolve possible race condition: " + raceBusterResult));
+                }
+              }
+
               conditionName =
                   String.format(
                       Locale.ROOT,
                       "%s(busy resources=%s)",
                       conditionName,
-                      StringJoinerKt.joinToString(busyResources, ","));
+                      busyResources != null
+                          ? StringJoinerKt.joinToString(busyResources, ",")
+                          : "null");
               break;
             default:
               break;
@@ -588,6 +628,7 @@ private IdleNotifier<IdleNotificationCallback> loopUntil(
     }
     return dynamicIdle;
   }
+
   @Override
   public void interruptEspressoTasks() {
     controllerHandler.post(

espresso/core/javatests/androidx/test/espresso/base/BUILD

@@ -236,7 +236,6 @@ axt_android_library_test(
         "//espresso/core/java/androidx/test/espresso/base:default_failure_handler",
         "//espresso/core/java/androidx/test/espresso/base:idling_resource_registry",
         "//ext/junit",
-        "//opensource/androidx:annotation",
         "//runner/android_junit_runner/java/androidx/test:runner",
         "//testapps/ui_testapp/java/androidx/test/ui/app:lib_exported",
         "//testapps/ui_testapp/javatests/androidx/test/ui/app:test_resources",

espresso/core/javatests/androidx/test/espresso/base/UiControllerImplTest.java

@@ -20,12 +20,17 @@
 import static junit.framework.Assert.assertTrue;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThrows;
 import static org.junit.Assert.fail;
 
 import android.os.Handler;
 import android.os.Looper;
 import android.os.Message;
+import android.os.SystemClock;
 import android.util.Log;
+import androidx.test.espresso.AppNotIdleException;
+import androidx.test.espresso.IdlingPolicies;
+import androidx.test.espresso.IdlingPolicy;
 import androidx.test.espresso.IdlingResourceTimeoutException;
 import androidx.test.espresso.base.IdlingResourceRegistry.IdleNotificationCallback;
 import androidx.test.ext.junit.runners.AndroidJUnit4;
@@ -371,6 +376,63 @@ public void run() {
     assertTrue("App should be idle.", latch.await(5, TimeUnit.SECONDS));
   }
 
+  @Test
+  public void loopMainThreadUntilIdle_racyIdleResource() throws InterruptedException {
+    IdlingPolicies.setMasterPolicyTimeout(10, TimeUnit.SECONDS);
+    OnDemandIdlingResource fakeResource = new OnDemandIdlingResource("FakeResource");
+    idlingResourceRegistry.registerResources(singletonList(fakeResource));
+    final CountDownLatch startedLatch = new CountDownLatch(1);
+    final CountDownLatch interruptedLatch = new CountDownLatch(1);
+    assertTrue(
+        testThread
+            .getHandler()
+            .post(
+                () -> {
+                  // This is the sequence of events we want:
+                  // - Resource starts busy.
+                  // - Resource becomes idle, event posted to queue to update idle registry.
+                  // - Timeout occurs, no more events will be processed.
+                  // - Espresso detects race condition while checking if the resource is idle.
+                  IdlingPolicy masterIdlingPolicy = IdlingPolicies.getMasterIdlingPolicy();
+                  long expectedTimeout =
+                      SystemClock.uptimeMillis()
+                          + masterIdlingPolicy
+                              .getIdleTimeoutUnit()
+                              .toMillis(masterIdlingPolicy.getIdleTimeout());
+                  testThread
+                      .getHandler()
+                      .postAtTime(
+                          () -> {
+                            Log.i(TAG, "Forcing resource to be idle.");
+                            fakeResource.forceIdleNow();
+                            // Busy wait until after the timeout to ensure race buster cannot run.
+                            try {
+                              Thread.sleep(200);
+                            } catch (InterruptedException e) {
+                              throw new RuntimeException(e);
+                            }
+                            assertTrue(
+                                "Sleep should bring us past the timeout.",
+                                SystemClock.uptimeMillis() > expectedTimeout);
+                          },
+                          expectedTimeout - 100);
+
+                  Log.i(TAG, "Hijacking thread and looping it.");
+                  startedLatch.countDown();
+                  assertThrows(
+                      "Expected for loopMainThreadUntilIdle to be interrupted",
+                      AppNotIdleException.class,
+                      () -> uiController.get().loopMainThreadUntilIdle());
+                  interruptedLatch.countDown();
+                }));
+
+    assertTrue("looper has started.", startedLatch.await(2, TimeUnit.SECONDS));
+    assertFalse(
+        "Should not have stopped looping the main thread yet!",
+        interruptedLatch.await(2, TimeUnit.SECONDS));
+    assertTrue("App should be interrupted.", interruptedLatch.await(20, TimeUnit.SECONDS));
+  }
+
   @Test
   public void loopMainThreadUntilIdle_multipleIdlingResources() throws InterruptedException {
     OnDemandIdlingResource fakeResource1 = new OnDemandIdlingResource("FakeResource1");