From f30847a58937945cd59665d7b8d9973f83922e90 Mon Sep 17 00:00:00 2001 From: Matt Carroll Date: Thu, 2 May 2019 23:10:23 -0700 Subject: [PATCH 1/2] Cause crash in FlutterJNI if invoked on non-main thread in debug mode (#31263). --- .../flutter/embedding/engine/FlutterJNI.java | 40 +++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java index 66e98087c4e5c..d355ecd05d266 100644 --- a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java +++ b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java @@ -7,6 +7,7 @@ import android.content.res.AssetManager; import android.graphics.Bitmap; import android.graphics.SurfaceTexture; +import android.os.Looper; import android.support.annotation.NonNull; import android.support.annotation.Nullable; import android.support.annotation.UiThread; @@ -17,6 +18,7 @@ import java.util.HashSet; import java.util.Set; +import io.flutter.BuildConfig; import io.flutter.embedding.engine.dart.PlatformMessageHandler; import io.flutter.embedding.engine.FlutterEngine.EngineLifecycleListener; import io.flutter.embedding.engine.renderer.FlutterRenderer; @@ -119,6 +121,7 @@ public class FlutterJNI { */ @UiThread public void setRenderSurface(@Nullable FlutterRenderer.RenderSurface renderSurface) { + ensureRunningOnMainThread(); this.renderSurface = renderSurface; } @@ -134,6 +137,7 @@ public void setRenderSurface(@Nullable FlutterRenderer.RenderSurface renderSurfa */ @UiThread public void setAccessibilityDelegate(@Nullable AccessibilityDelegate accessibilityDelegate) { + ensureRunningOnMainThread(); this.accessibilityDelegate = accessibilityDelegate; } @@ -146,6 +150,7 @@ public void setAccessibilityDelegate(@Nullable AccessibilityDelegate accessibili @SuppressWarnings("unused") @UiThread private void updateSemantics(ByteBuffer buffer, String[] strings) { + ensureRunningOnMainThread(); if (accessibilityDelegate != null) { accessibilityDelegate.updateSemantics(buffer, strings); } @@ -163,6 +168,7 @@ private void updateSemantics(ByteBuffer buffer, String[] strings) { @SuppressWarnings("unused") @UiThread private void updateCustomAccessibilityActions(ByteBuffer buffer, String[] strings) { + ensureRunningOnMainThread(); if (accessibilityDelegate != null) { accessibilityDelegate.updateCustomAccessibilityActions(buffer, strings); } @@ -173,6 +179,7 @@ private void updateCustomAccessibilityActions(ByteBuffer buffer, String[] string @SuppressWarnings("unused") @UiThread private void onFirstFrame() { + ensureRunningOnMainThread(); if (renderSurface != null) { renderSurface.onFirstFrameRendered(); } @@ -209,6 +216,7 @@ private void onFirstFrame() { */ @UiThread public void setPlatformMessageHandler(@Nullable PlatformMessageHandler platformMessageHandler) { + ensureRunningOnMainThread(); this.platformMessageHandler = platformMessageHandler; } @@ -232,21 +240,25 @@ private void handlePlatformMessageResponse(int replyId, byte[] reply) { @UiThread public void addEngineLifecycleListener(@NonNull EngineLifecycleListener engineLifecycleListener) { + ensureRunningOnMainThread(); engineLifecycleListeners.add(engineLifecycleListener); } @UiThread public void removeEngineLifecycleListener(@NonNull EngineLifecycleListener engineLifecycleListener) { + ensureRunningOnMainThread(); engineLifecycleListeners.remove(engineLifecycleListener); } @UiThread public void addOnFirstFrameRenderedListener(@NonNull OnFirstFrameRenderedListener listener) { + ensureRunningOnMainThread(); firstFrameListeners.add(listener); } @UiThread public void removeOnFirstFrameRenderedListener(@NonNull OnFirstFrameRenderedListener listener) { + ensureRunningOnMainThread(); firstFrameListeners.remove(listener); } @@ -254,6 +266,7 @@ public void removeOnFirstFrameRenderedListener(@NonNull OnFirstFrameRenderedList //----- Start from FlutterView ----- @UiThread public void onSurfaceCreated(@NonNull Surface surface) { + ensureRunningOnMainThread(); ensureAttachedToNative(); nativeSurfaceCreated(nativePlatformViewId, surface); } @@ -262,6 +275,7 @@ public void onSurfaceCreated(@NonNull Surface surface) { @UiThread public void onSurfaceChanged(int width, int height) { + ensureRunningOnMainThread(); ensureAttachedToNative(); nativeSurfaceChanged(nativePlatformViewId, width, height); } @@ -270,6 +284,7 @@ public void onSurfaceChanged(int width, int height) { @UiThread public void onSurfaceDestroyed() { + ensureRunningOnMainThread(); ensureAttachedToNative(); nativeSurfaceDestroyed(nativePlatformViewId); } @@ -290,6 +305,7 @@ public void setViewportMetrics( int physicalViewInsetBottom, int physicalViewInsetLeft ) { + ensureRunningOnMainThread(); ensureAttachedToNative(); nativeSetViewportMetrics( nativePlatformViewId, @@ -324,6 +340,7 @@ private native void nativeSetViewportMetrics( @UiThread public Bitmap getBitmap() { + ensureRunningOnMainThread(); ensureAttachedToNative(); return nativeGetBitmap(nativePlatformViewId); } @@ -332,6 +349,7 @@ public Bitmap getBitmap() { @UiThread public void dispatchPointerDataPacket(ByteBuffer buffer, int position) { + ensureRunningOnMainThread(); ensureAttachedToNative(); nativeDispatchPointerDataPacket(nativePlatformViewId, buffer, position); } @@ -358,6 +376,7 @@ public void dispatchSemanticsAction(int id, @NonNull AccessibilityBridge.Action @UiThread public void dispatchSemanticsAction(int id, int action, ByteBuffer args, int argsPosition) { + ensureRunningOnMainThread(); ensureAttachedToNative(); nativeDispatchSemanticsAction(nativePlatformViewId, id, action, args, argsPosition); } @@ -372,6 +391,7 @@ private native void nativeDispatchSemanticsAction( @UiThread public void setSemanticsEnabled(boolean enabled) { + ensureRunningOnMainThread(); ensureAttachedToNative(); nativeSetSemanticsEnabled(nativePlatformViewId, enabled); } @@ -380,6 +400,7 @@ public void setSemanticsEnabled(boolean enabled) { @UiThread public void setAccessibilityFeatures(int flags) { + ensureRunningOnMainThread(); ensureAttachedToNative(); nativeSetAccessibilityFeatures(nativePlatformViewId, flags); } @@ -388,6 +409,7 @@ public void setAccessibilityFeatures(int flags) { @UiThread public void registerTexture(long textureId, SurfaceTexture surfaceTexture) { + ensureRunningOnMainThread(); ensureAttachedToNative(); nativeRegisterTexture(nativePlatformViewId, textureId, surfaceTexture); } @@ -396,6 +418,7 @@ public void registerTexture(long textureId, SurfaceTexture surfaceTexture) { @UiThread public void markTextureFrameAvailable(long textureId) { + ensureRunningOnMainThread(); ensureAttachedToNative(); nativeMarkTextureFrameAvailable(nativePlatformViewId, textureId); } @@ -404,6 +427,7 @@ public void markTextureFrameAvailable(long textureId) { @UiThread public void unregisterTexture(long textureId) { + ensureRunningOnMainThread(); ensureAttachedToNative(); nativeUnregisterTexture(nativePlatformViewId, textureId); } @@ -419,6 +443,7 @@ public boolean isAttached() { @UiThread public void attachToNative(boolean isBackgroundView) { + ensureRunningOnMainThread(); ensureNotAttachedToNative(); nativePlatformViewId = nativeAttach(this, isBackgroundView); } @@ -427,6 +452,7 @@ public void attachToNative(boolean isBackgroundView) { @UiThread public void detachFromNativeAndReleaseResources() { + ensureRunningOnMainThread(); ensureAttachedToNative(); nativeDestroy(nativePlatformViewId); nativePlatformViewId = null; @@ -441,6 +467,7 @@ public void runBundleAndSnapshotFromLibrary( @Nullable String pathToEntrypointFunction, @NonNull AssetManager assetManager ) { + ensureRunningOnMainThread(); ensureAttachedToNative(); nativeRunBundleAndSnapshotFromLibrary( nativePlatformViewId, @@ -461,6 +488,7 @@ private native void nativeRunBundleAndSnapshotFromLibrary( @UiThread public void dispatchEmptyPlatformMessage(String channel, int responseId) { + ensureRunningOnMainThread(); if (isAttached()) { nativeDispatchEmptyPlatformMessage(nativePlatformViewId, channel, responseId); } else { @@ -477,6 +505,7 @@ private native void nativeDispatchEmptyPlatformMessage( @UiThread public void dispatchPlatformMessage(String channel, ByteBuffer message, int position, int responseId) { + ensureRunningOnMainThread(); if (isAttached()) { nativeDispatchPlatformMessage( nativePlatformViewId, @@ -501,6 +530,7 @@ private native void nativeDispatchPlatformMessage( @UiThread public void invokePlatformMessageEmptyResponseCallback(int responseId) { + ensureRunningOnMainThread(); if (isAttached()) { nativeInvokePlatformMessageEmptyResponseCallback(nativePlatformViewId, responseId); } else { @@ -516,6 +546,7 @@ private native void nativeInvokePlatformMessageEmptyResponseCallback( @UiThread public void invokePlatformMessageResponseCallback(int responseId, ByteBuffer message, int position) { + ensureRunningOnMainThread(); if (isAttached()) { nativeInvokePlatformMessageResponseCallback( nativePlatformViewId, @@ -560,6 +591,15 @@ private void ensureAttachedToNative() { } } + private void ensureRunningOnMainThread() { + if (BuildConfig.DEBUG && Looper.myLooper() != Looper.getMainLooper()) { + throw new RuntimeException( + "Attempted to invoke a @UiThread method on a different thread (" + + Thread.currentThread().getName() + ")" + ); + } + } + /** * Delegate responsible for creating and updating Android-side caches of Flutter's semantics * tree and custom accessibility actions. From a5bef04099ea35c3073bbed9987dfe3c28fc4e8b Mon Sep 17 00:00:00 2001 From: Matt Carroll Date: Fri, 3 May 2019 13:57:23 -0700 Subject: [PATCH 2/2] Changed exception to throw all the time. Updated message. --- .../io/flutter/embedding/engine/FlutterJNI.java | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java index d355ecd05d266..c5d92e1752f26 100644 --- a/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java +++ b/shell/platform/android/io/flutter/embedding/engine/FlutterJNI.java @@ -104,6 +104,13 @@ public class FlutterJNI { private PlatformMessageHandler platformMessageHandler; private final Set engineLifecycleListeners = new HashSet<>(); private final Set firstFrameListeners = new HashSet<>(); + private final Looper mainLooper; // cached to avoid synchronization on repeat access. + + 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(); + } /** * Sets the {@link FlutterRenderer.RenderSurface} delegate for the attached Flutter context. @@ -592,10 +599,10 @@ private void ensureAttachedToNative() { } private void ensureRunningOnMainThread() { - if (BuildConfig.DEBUG && Looper.myLooper() != Looper.getMainLooper()) { + if (Looper.myLooper() != mainLooper) { throw new RuntimeException( - "Attempted to invoke a @UiThread method on a different thread (" - + Thread.currentThread().getName() + ")" + "Methods marked with @UiThread must be executed on the main thread. Current thread: " + + Thread.currentThread().getName() ); } }