[android] Fixes 'drawRenderNode called on a context with no surface' crash #33655

0xZOne · 2022-05-27T08:19:25Z

When a memory pressure warning is received and the level equal {@code ComponentCallbacks2.TRIM_MEMORY_COMPLETE}, the Android system release the underlying surface. If we continue to use the surface (e.g., call lockHardwareCanvas), a crash occurs, and we found that this crash appeared on Android 10 and above.

Steps to reproduce:

Open a PlatformView page in your app;
Switch your app to the background;
Simulate low memory : adb shell am send-trim-memory <your_package_name_here> COMPLETE;
Bring your app to the foreground;
Then crash

Here our workaround is to recreate the surface before using it.

Fixes flutter/flutter#103870

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide and the C++, Objective-C, Java style guides.
I listed at least one issue that this PR fixes in the description above.
I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test-exempt. See testing the engine for instructions on
writing and running engine tests.
I updated/added relevant documentation (doc comments with ///).
I signed the CLA.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

0xZOne

The systems we can reproduce locally are Android 10, 11, and 12.
Android 8.1 and 9 are only present in the crash info collection system, probably for different reasons.

dnfield · 2022-06-02T20:12:25Z

shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java

+                if (!flutterJNI.isAttached()) {
+                  return;
+                }
+                Log.v(TAG, "Releasing a SurfaceTexture (" + id + ").");


Please remove this log - it's not really useful to end developers and will just end up adding to noise.

Considering the complexity of the finalize method, e.g., resurrection, I do not modify its logic here.

I'll try to replace the finalize method with PhantomReferece in combination with ReferenceQueue in a new PR later.

blasten · 2022-06-02T20:56:01Z

shell/platform/android/io/flutter/embedding/android/FlutterActivityAndFragmentDelegate.java

@@ -860,6 +862,7 @@ void onLowMemory() {
    ensureAlive();
    flutterEngine.getDartExecutor().notifyLowMemoryWarning();
    flutterEngine.getSystemChannel().sendMemoryPressureWarning();
+    flutterEngine.getRenderer().onTrimMemory(TRIM_MEMORY_COMPLETE);


weird. there's no call to delegate.onLowMemory from io.flutter.embedding.android.FlutterActivity, but there's one from io.flutter.embedding.android.FlutterFragment. This seems wrong. Do we know why that's the case? cc @dnfield

Activity is using onTrimMemory.

In general, onTrimMemory is preferable over onLowMemory - whenever onLowMemory will get called, onTrimMemory will also get called (right before or right after, can't remember).

According to the Android documentation, onLowMemory is only used for API levels lower than 14:

To support API levels lower than 14, you can use the ComponentCallbacks.onLowMemory() method as a fallback that's roughly equivalent to the ComponentCallbacks2#TRIM_MEMORY_COMPLETE level.

Maybe we should override onLowMemory in io.flutter.embedding.android.FlutterActivity class.

It's confusing that onLowMemory is called from FlutterFragment but not FlutterActivity. We don't support API level 14, so should we remove all instances of onLowMemory?

It's confusing that onLowMemory is called from FlutterFragment but not FlutterActivity. We don't support API level 14, so should we remove all instances of onLowMemory?

done.

0xZOne

remove the unnecessary log

blasten · 2022-06-03T17:39:20Z

shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java

@@ -200,34 +241,30 @@ protected void finalize() throws Throwable {
          return;
        }

-        handler.post(new SurfaceTextureFinalizerRunnable(id, flutterJNI));
+        handler.post(


@ColdPaleLight, @jason-simmons I was going over the PR ce8fe8b

Can you comment on why the PR posted a task to the main looper instead of just running directly?

Because 'unregisterTexture' must be called on the main thread. And 'finalize' method is called on 'Finalizer' thread.

it looks like there's a race when reading released, or when unregisterTexture is called?

Ahh, I see, here we need to make sure that the finalizer thread can see changes of released value (for example: use keyword volatile). Or we can defer released check to the main thread. WDYT?

sure, deferring sounds good. Also, can we add comments about what is happening here, why we need all of this? and clarify that this is a case where someone forgot to do the right thing. flutter/flutter#88571

The finalize() execution is not guaranteed. Can we consider using ReferenceQueue here?

blasten · 2022-06-03T17:51:40Z

shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java

@@ -44,6 +46,10 @@ public class FlutterRenderer implements TextureRegistry {
  private boolean isDisplayingFlutterUi = false;
  private Handler handler = new Handler();

+  @NonNull
+  private final Set<TextureRegistry.OnLowMemoryListener> onLowMemoryListeners =
+      new CopyOnWriteArraySet<>();


is this being accessed from different threads?

this is still unresolved

This one has been solved.

blasten · 2022-06-03T17:52:30Z

shell/platform/android/io/flutter/plugin/platform/PlatformViewWrapper.java

+          // surface. If we continue to use the surface (e.g., call lockHardwareCanvas), a crash
+          // occurs, and we found that this crash appeared on Android10 and above.
+          //
+          // Here our workaround is to recreate the surface before using it.


nit: also point to the issue this is fixing

blasten · 2022-06-03T17:55:51Z

shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java

@@ -91,6 +97,19 @@ public void removeIsDisplayingFlutterUiListener(@NonNull FlutterUiDisplayListene
    flutterJNI.removeIsDisplayingFlutterUiListener(listener);
  }

+  /** Adds a listener that is invoked when a memory pressure warning was forward. */
+  public void addOnLowMemoryListener(@NonNull OnLowMemoryListener listener) {


can we make this private or package-only, and add VisibleForTesting?

blasten · 2022-06-03T17:56:10Z

shell/platform/android/io/flutter/embedding/engine/renderer/FlutterRenderer.java

+   * Removes a {@link OnLowMemoryListener} that was added with {@link
+   * #addOnLowMemoryListener(OnLowMemoryListener)}.
+   */
+  public void removeOnLowMemoryListener(@NonNull OnLowMemoryListener listener) {


same comment about visibility

blasten · 2022-06-03T18:04:29Z

shell/platform/android/io/flutter/view/TextureRegistry.java

+  /** Listener invoked when a memory pressure warning was forward. */
+  interface OnLowMemoryListener {
+    /** This method will be invoked when a memory pressure warning was forward. */
+    void onLowMemory(int level);


can this be called onTrimMemory?

blasten · 2022-06-03T18:04:54Z

shell/platform/android/io/flutter/view/TextureRegistry.java

@@ -55,4 +65,10 @@ interface OnFrameConsumedListener {
     */
    void onFrameConsumed();
  }
+
+  /** Listener invoked when a memory pressure warning was forward. */
+  interface OnLowMemoryListener {


can this be called OnTrimMemoryListener?

blasten · 2022-06-03T18:05:38Z

shell/platform/android/io/flutter/view/TextureRegistry.java

@@ -45,6 +52,9 @@ interface SurfaceTextureEntry {

    /** Set a listener that will be notified when the most recent image has been consumed. */
    default void setOnFrameConsumedListener(@Nullable OnFrameConsumedListener listener) {}
+
+    /** Set a listener that will be notified when a memory pressure warning was forward. */
+    default void setOnLowMemoryListener(@Nullable OnLowMemoryListener listener) {}


can this be called setOnTrimMemoryListener?

flutter-dashboard · 2022-06-06T08:18:04Z

This pull request has been changed to a draft. The currently pending flutter-gold status will not be able to resolve until a new commit is pushed or the change is marked ready for review again.

…urface' crash (flutter/engine#33655)

GabrielBB · 2022-06-15T10:50:59Z

Definately this commit is must needed as hotfix. @dnfield Could you forward this as hotfix to stable, 'cause it affects lots of low end devices with crashes.

@dnfield Any plans to send this to hotfix this week ? 👀

dnfield · 2022-06-15T16:41:54Z

I think it would be good to at least see this pass Google's internal testing before requesting a hotfix for it.

…crash (flutter#33655)

dnfield · 2022-06-21T18:20:49Z

This is undergoing testing internally via cl/456284311

If that lands without issue I'll file a CP request.

dnfield · 2022-06-28T17:28:59Z

This has passed internal tests - filed flutter/flutter#106746

…crash (flutter#33655)

…crash (flutter#33655) Fixes cherry-pick conflict for deleted test, see flutter@031fa76#diff-553d8d6ccf46421df54ff1b840606a323494ca2c73f3ce1731169c4d44504b2fL866

…crash (#33655) (#34400) Fixes cherry-pick conflict for deleted test, see 031fa76#diff-553d8d6ccf46421df54ff1b840606a323494ca2c73f3ce1731169c4d44504b2fL866 Co-authored-by: Rulong Chen（陈汝龙） <rulong.crl@alibaba-inc.com>

cclink · 2022-07-01T15:06:02Z

When we could get this fix in stable channel?

dnfield · 2022-07-01T16:08:09Z

The cherry pick has landed, but it will probably be a little bit at this point before it's propagated (next week is a US holiday and many people are taking vacation).

CaseyHillers · 2022-07-01T16:11:57Z

This shipped an hour ago in the 3.0.4 stable. flutter upgrade will pull this fix in.

dnfield · 2022-07-01T16:17:53Z

Oh, sorry, forgot that @CaseyHillers was on the job and he gets stuff done.

Paitomax · 2022-07-01T16:25:23Z

I tested with version 3.0.4 and everything is fine, the error does not happen.

@0xZOne and everyone else who worked on this fix, thanks.

raywoocn · 2022-09-23T12:09:44Z

The problem is still there with flutter V3.3.2.

weicheng59 · 2022-10-14T10:14:24Z

I also experienced this problem with 3.0.4，3.0.5，3.3.4. It's not reproducible with
Simulate low memory : adb shell am send-trim-memory <your_package_name_here> COMPLETE;
for most of our test phones but reproducible 100% with vivo x80 phone(android 12) without the need to simulate low memory.
We also notice around 2% of our user are having the same exception from bugly.
It happened with both Webview and Platformview.
I am willing to provide more details if anyone want to help out because this issue block our android release.

pakeng · 2023-01-09T09:47:19Z

@weicheng59 你们解决了这个问题吗

pakeng · 2023-01-09T09:48:50Z

@raywoocn Is any fix ？

weicheng59 · 2023-01-09T15:06:22Z

@pakeng we avoid it by removing the platform view when the app go to background and rebuild it when the app go to foreground. You can get messages of app going background and foreground by having android native code send them to you. After this work around, no more crashes.

flutter-dashboard bot added the platform-android label May 27, 2022

0xZOne requested review from blasten and ColdPaleLight May 27, 2022 08:33

0xZOne force-pushed the task/issue_103870 branch 3 times, most recently from a9813bd to c6c0cae Compare May 27, 2022 09:46

ColdPaleLight removed their request for review May 27, 2022 11:24

0xZOne force-pushed the task/issue_103870 branch from c6c0cae to e2b0519 Compare May 27, 2022 12:08

0xZOne mentioned this pull request May 30, 2022

Android Crash (live app release mode) signal 6 (SIGABRT), code -1 (SI_QUEUE) flutter/flutter#104514

Closed

0xZOne commented Jun 1, 2022

View reviewed changes

dnfield reviewed Jun 2, 2022

View reviewed changes

blasten reviewed Jun 2, 2022

View reviewed changes

0xZOne commented Jun 2, 2022

View reviewed changes

0xZOne requested review from blasten and dnfield June 2, 2022 23:40

blasten reviewed Jun 3, 2022

View reviewed changes

0xZOne force-pushed the task/issue_103870 branch 2 times, most recently from a6179a0 to 96e6410 Compare June 4, 2022 17:59

0xZOne requested a review from blasten June 4, 2022 18:12

0xZOne marked this pull request as draft June 6, 2022 08:13

0xZOne force-pushed the task/issue_103870 branch from ff0ff8a to e238645 Compare June 6, 2022 09:25

canewsin mentioned this pull request Jun 6, 2022

Android Crashes on Certain API 30 Devices flutter/flutter#105384

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 13, 2022

031fa76 [android] Fixes 'drawRenderNode called on a context with no s…

8fa1e35

…urface' crash (flutter/engine#33655)

engine-flutter-autoroll mentioned this pull request Jun 13, 2022

Roll Flutter Engine from 6670f92130d9 to 7aa848494c6c (10 revisions) flutter/flutter#105916

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 13, 2022

031fa76 [android] Fixes 'drawRenderNode called on a context with no s…

4a493e2

…urface' crash (flutter/engine#33655)

engine-flutter-autoroll mentioned this pull request Jun 13, 2022

Roll Flutter Engine from 6670f92130d9 to 8cee666b48eb (11 revisions) flutter/flutter#105918

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 13, 2022

031fa76 [android] Fixes 'drawRenderNode called on a context with no s…

6511140

…urface' crash (flutter/engine#33655)

engine-flutter-autoroll mentioned this pull request Jun 13, 2022

Roll Flutter Engine from 6670f92130d9 to d792826365d7 (12 revisions) flutter/flutter#105922

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 13, 2022

031fa76 [android] Fixes 'drawRenderNode called on a context with no s…

d971792

…urface' crash (flutter/engine#33655)

engine-flutter-autoroll mentioned this pull request Jun 13, 2022

Roll Flutter Engine from 6670f92130d9 to 8cf55ccef913 (13 revisions) flutter/flutter#105928

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 13, 2022

031fa76 [android] Fixes 'drawRenderNode called on a context with no s…

e6c3e47

…urface' crash (flutter/engine#33655)

engine-flutter-autoroll mentioned this pull request Jun 14, 2022

Roll Flutter Engine from 6670f92130d9 to edc2aa4e894b (14 revisions) flutter/flutter#105934

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jun 14, 2022

031fa76 [android] Fixes 'drawRenderNode called on a context with no s…

ee63fed

…urface' crash (flutter/engine#33655)

houhuayong pushed a commit to houhuayong/engine that referenced this pull request Jun 21, 2022

[android] Fixes 'drawRenderNode called on a context with no surface' …

ccb2d2c

…crash (flutter#33655)

CaseyHillers pushed a commit to CaseyHillers/engine that referenced this pull request Jun 29, 2022

[android] Fixes 'drawRenderNode called on a context with no surface' …

59e94cb

…crash (flutter#33655)

0xZOne mentioned this pull request Jul 7, 2022

感谢你对platformview的贡献 0xZOne/platform_view_test#1

Closed

[android] Fixes 'drawRenderNode called on a context with no surface' crash #33655

[android] Fixes 'drawRenderNode called on a context with no surface' crash #33655

Uh oh!

Conversation

0xZOne commented May 27, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

0xZOne left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0xZOne left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

blasten Jun 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

flutter-dashboard bot commented Jun 6, 2022

Uh oh!

0xZOne commented May 27, 2022 •

edited

Loading

blasten Jun 3, 2022 •

edited

Loading