Add lock

Author: Emmanuel Garcia

shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java

@@ -42,6 +42,7 @@
 import java.util.Locale;
 import java.util.Set;
 import java.util.concurrent.CopyOnWriteArraySet;
+import java.util.concurrent.atomic.AtomicReference;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
 
 /**
@@ -73,7 +74,7 @@
  *
  * <pre>{@code
  * // Instantiate FlutterJNI and attach to the native side.
- * FlutterJNI flutterJNI = new FlutterJNI();
+ * FlutterJNI flutterJNI = new FlutterJNI(Looper.getMainLooper());
  * flutterJNI.attachToNative();
  *
  * // Use FlutterJNI as desired. flutterJNI.dispatchPointerDataPacket(...);
@@ -107,11 +108,13 @@ public class FlutterJNI {
   // platform thread and doesn't require locking.
   private ReentrantReadWriteLock shellHolderLock = new ReentrantReadWriteLock();
 
-  // Prefer using the FlutterJNI.Factory so it's easier to test.
+  /** @param looper The main looper. Typically, Looper.getMainLooper(). */
+  public FlutterJNI(@NonNull Looper looper) {
+    mainLooper = looper;
+  }
+
   public FlutterJNI() {
-    // We cache the main looper so that we can ensure calls are made on the main thread
-    // without consistently paying the synchronization cost of getMainLooper().
-    mainLooper = Looper.getMainLooper();
+    this(Looper.getMainLooper());
   }
 
   /**
@@ -1170,8 +1173,7 @@ public FlutterOverlaySurface createOverlaySurface() {
   }
 
   @SuppressWarnings("unused")
-  @UiThread
-  public void destroyOverlaySurfaces() {
+  public void destroyOverlaySurfaces() throws Exception {
     // Normally, this method is called on the Android main thread.
     //
     // However, it may be called from a different thread after the thread merger lease expired
@@ -1185,14 +1187,15 @@ public void destroyOverlaySurfaces() {
     // method hops to the Android main thread if necessary.
     //
     // See Rasterizer::Teardown in C++, and FlutterEngineCache in Java.
-    runOnMainThread(
+    runOnLooper(
         () -> {
           if (platformViewsController == null) {
             throw new RuntimeException(
                 "platformViewsController must be set before attempting to destroy an overlay surface");
           }
           platformViewsController.destroyOverlaySurfaces();
-        });
+        },
+        mainLooper);
   }
   // ----- End Engine Lifecycle Support ----
 
@@ -1423,18 +1426,39 @@ private void ensureRunningOnMainThread() {
   }
 
   /**
-   * Causes the runnable r to be run on the Android main thread. If the current thread is different,
-   * then it adds the runnable to the message queue of the main thread.
+   * Causes the runnable r to be run on the thread associated with the given looper l. Then, it
+   * waits for the runnable to run.
+   *
+   * <p>If the runnable throws, then the exception is captured and rethrown in the current thread.
    *
    * @param r The runnable that will be executed.
+   * @param l The looper where the runnable is added.
    */
-  private void runOnMainThread(Runnable r) {
-    if (Looper.myLooper() == mainLooper) {
+  private void runOnLooper(Runnable r, Looper l) throws Exception {
+    if (Looper.myLooper() == l) {
       r.run();
       return;
     }
-    final Handler handler = new Handler(mainLooper);
-    handler.post(r);
+    final AtomicReference<Exception> exception = new AtomicReference<>();
+    final Handler handler = new Handler(l);
+    handler.post(
+        () -> {
+          try {
+            r.run();
+          } catch (Exception e) {
+            exception.set(e);
+          }
+          synchronized (handler) {
+            handler.notify();
+          }
+        });
+    synchronized (handler) {
+      handler.wait();
+    }
+    if (exception.get() != null) {
+      // Rethrow the exception on the current thread.
+      throw exception.get();
+    }
   }
 
   /**

shell/platform/android/test/io/flutter/embedding/engine/FlutterJNITest.java

@@ -1,19 +1,19 @@
 package io.flutter.embedding.engine;
 
-import static android.os.Looper.getMainLooper;
 import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.fail;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
-import static org.robolectric.Shadows.shadowOf;
 
 import android.annotation.TargetApi;
 import android.content.Context;
 import android.content.res.Configuration;
 import android.content.res.Resources;
+import android.os.Handler;
 import android.os.LocaleList;
+import android.os.Looper;
+import android.os.Message;
 import io.flutter.embedding.engine.dart.DartExecutor;
 import io.flutter.embedding.engine.mutatorsstack.FlutterMutatorsStack;
 import io.flutter.embedding.engine.renderer.FlutterUiDisplayListener;
@@ -22,7 +22,6 @@
 import io.flutter.plugin.platform.PlatformViewsController;
 import java.nio.ByteBuffer;
 import java.util.Locale;
-import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicReference;
 import org.junit.Test;
@@ -253,33 +252,45 @@ public void invokePlatformMessageResponseCallback__wantsDirectBuffer() {
 
   @Test
   public void destroyOverlaySurfaces__calledFromDifferentThread() throws Exception {
+    AtomicReference<FlutterJNI> flutterJNI = new AtomicReference<>();
     PlatformViewsController platformViewsController = mock(PlatformViewsController.class);
 
-    FlutterJNI flutterJNI = new FlutterJNI();
-    flutterJNI.setPlatformViewsController(platformViewsController);
-
-    AtomicReference<Exception> exception = new AtomicReference<>();
-
-    // --- Execute Test ---
-    CountDownLatch latch = new CountDownLatch(1);
-    Thread thread =
+    Object lock = new Object();
+    // This thread acts as the Android main thread.
+    // This is required because the test itself runs on the main thread, and without this
+    // workaround,
+    // a deadlock is produced.
+    Thread mainThreadImpostor =
         new Thread(
             () -> {
-              try {
-                flutterJNI.destroyOverlaySurfaces();
-              } catch (Exception e) {
-                exception.set(e);
+              Looper.prepare();
+
+              flutterJNI.set(new FlutterJNI(Looper.myLooper()));
+              flutterJNI.get().setPlatformViewsController(platformViewsController);
+
+              synchronized (lock) {
+                lock.notify();
               }
-              latch.countDown();
+
+              new Handler(Looper.myLooper()) {
+                public void handleMessage(Message msg) {
+                  msg.getCallback().run();
+                }
+              };
+              Looper.loop();
             });
 
-    thread.start();
-    latch.await();
-    shadowOf(getMainLooper()).idle();
+    mainThreadImpostor.start();
+    synchronized (lock) {
+      lock.wait();
+    }
+
+    // --- Execute Test ---
+    flutterJNI.get().destroyOverlaySurfaces();
 
     // --- Verify Results ---
-    if (exception.get() != null) {
-      fail("Unexpected exception: " + exception.get().toString());
-    }
+    verify(platformViewsController, times(1)).destroyOverlaySurfaces();
+
+    mainThreadImpostor.interrupt();
   }
 }